1694104 - Crash in [@ kobs4j_d.dll]

Status: RESOLVED FIXED

(Core :: Printing: Setup, defect)

Description

[Gabriele Svelto [:gsvelto] (PTO)]

Crash report: https://crash-stats.mozilla.org/report/index/5c35436f-a8d5-4d82-96fa-9bde00210219

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 1 frames of crashing thread:

0 kobs4j_d.dll kobs4j_d.dll@0x35fe2b 

I think this comes from some Konica Minolta printer drivers back from 2005. I've sampled a few crashes and the DLL version seem to be 1.0.0.1.

I extracted a better stack trace of the crashing thread:

 0  KOBS4J_D.DLL!<unknown in kobs4j_d.dll> + 0x24f2b1
 1  KOBS4J_D.DLL!Prc_MergeDMode + 0x1d
 2  KOBS4J_C.DLL!<unknown in kobs4j_c.dll> + 0x16473
 3  KOBS4J_C.DLL!<unknown in kobs4j_c.dll> + 0xb060
 4  KOBS4J_C.DLL!<unknown in kobs4j_c.dll> + 0xe93e
 5  KOBS4J_C.DLL!<unknown in kobs4j_c.dll> + 0xd794
 6  winspool.drv!DocumentPropertySheets + 0xc6
 7  winspool.drv!long DocumentPropertiesWNative(struct HWND__*,void *,unsigned short *,struct _devicemodeW *,unsigned long,struct _devicemodeW *,unsigned long) + 0xce
 8  winspool.drv!DocumentPropertiesW + 0x8f
 9  xul.dll!nsPrinterWin::SupportsMonochrome() const [nsPrinterWin.cpp:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 151 + 0x19]
10  xul.dll!mozilla::SpawnPrintBackgroundTask<nsPrinterBase,bool>::<unnamed-tag>::operator()() const [PrintBackgroundTask.h:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 53 + 0x13]
11  xul.dll!mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/widget/PrintBackgroundTask.h:48:11'>::Run() [nsThreadUtils.h:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 534 + 0x9]
12  xul.dll!nsThreadPool::Run() [nsThreadPool.cpp:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 301 + 0x10]
13  xul.dll!nsThread::ProcessNextEvent(bool, bool*) [nsThread.cpp:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 1200 + 0x22]
14  xul.dll!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) [MessagePump.cpp:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 332 + 0x28]
15  xul.dll!MessageLoop::RunHandler() [message_loop.cc:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 327 + 0x16]
16  xul.dll!MessageLoop::Run() [message_loop.cc:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 309 + 0x5]
17  xul.dll!static nsThread::ThreadFunc(void*) [nsThread.cpp:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 441 + 0x8]
18  nss3.dll!_PR_NativeRunThread(void*) [pruthr.c:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 399 + 0xe]
19  nss3.dll!pr_root(void*) [w95thred.c:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 139 + 0xd]
20  ucrtbase.dll!thread_start<unsigned int (__cdecl*)(void *),1> + 0x42

Comment 1

[Toshihito Kikuchi [:toshi]]

Moving to the Printing component for triage.

Comment 3

[Gabriele Svelto [:gsvelto] (PTO)]

The volume here is crazy! We're looking at up to ~10k affected users per day. Is there something we can do about this?

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Bumping up the severity.

From a duplicated bug comment;

FireFox crashes. Figured out that this is related to the name of our network printer with special characters: Värmdö. When deleting this printer in about:config, printing works

:jwatt, :bobowen, any insights on this?

Comment 5

[Bob Owen (:bobowen)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)

Bumping up the severity.

From a duplicated bug comment;

FireFox crashes. Figured out that this is related to the name of our network printer with special characters: Värmdö. When deleting this printer in about:config, printing works

:jwatt, :bobowen, any insights on this?

I think this is all down to us pulling back various settings and capabilities for a printer in many threads.
In a previous bug I changed it so we lock around getting the default DEVMODE, but it looks like we can't rely on these drivers being at all thread-safe despite the fact that the OpenPrinter docs suggest you can if you have different printer handles per thread.

So, I think we should add another Mutex (possibly per printer name, rather than nsIPrinter object, although maybe that's overkill) and lock around probably the following: DeviceCapabilitiesW, DocumentPropertiesW and CreateICW/DCW

I think OpenPrinterW itself is probably OK (especially as we don't pass in any defaults) and also GetDeviceCaps, as we're just pulling information from an already created information/device context.

Comment 6

[andreas]

(In reply to Gabriele Svelto [:gsvelto] from comment #0)

Crash report: https://crash-stats.mozilla.org/report/index/5c35436f-a8d5-4d82-96fa-9bde00210219

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 1 frames of crashing thread:

0 kobs4j_d.dll kobs4j_d.dll@0x35fe2b 

I think this comes from some Konica Minolta printer drivers back from 2005. I've sampled a few crashes and the DLL version seem to be 1.0.0.1.

I extracted a better stack trace of the crashing thread:

 0  KOBS4J_D.DLL!<unknown in kobs4j_d.dll> + 0x24f2b1
 1  KOBS4J_D.DLL!Prc_MergeDMode + 0x1d
 2  KOBS4J_C.DLL!<unknown in kobs4j_c.dll> + 0x16473
 3  KOBS4J_C.DLL!<unknown in kobs4j_c.dll> + 0xb060
 4  KOBS4J_C.DLL!<unknown in kobs4j_c.dll> + 0xe93e
 5  KOBS4J_C.DLL!<unknown in kobs4j_c.dll> + 0xd794
 6  winspool.drv!DocumentPropertySheets + 0xc6
 7  winspool.drv!long DocumentPropertiesWNative(struct HWND__*,void *,unsigned short *,struct _devicemodeW *,unsigned long,struct _devicemodeW *,unsigned long) + 0xce
 8  winspool.drv!DocumentPropertiesW + 0x8f
 9  xul.dll!nsPrinterWin::SupportsMonochrome() const [nsPrinterWin.cpp:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 151 + 0x19]
10  xul.dll!mozilla::SpawnPrintBackgroundTask<nsPrinterBase,bool>::<unnamed-tag>::operator()() const [PrintBackgroundTask.h:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 53 + 0x13]
11  xul.dll!mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/widget/PrintBackgroundTask.h:48:11'>::Run() [nsThreadUtils.h:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 534 + 0x9]
12  xul.dll!nsThreadPool::Run() [nsThreadPool.cpp:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 301 + 0x10]
13  xul.dll!nsThread::ProcessNextEvent(bool, bool*) [nsThread.cpp:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 1200 + 0x22]
14  xul.dll!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) [MessagePump.cpp:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 332 + 0x28]
15  xul.dll!MessageLoop::RunHandler() [message_loop.cc:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 327 + 0x16]
16  xul.dll!MessageLoop::Run() [message_loop.cc:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 309 + 0x5]
17  xul.dll!static nsThread::ThreadFunc(void*) [nsThread.cpp:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 441 + 0x8]
18  nss3.dll!_PR_NativeRunThread(void*) [pruthr.c:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 399 + 0xe]
19  nss3.dll!pr_root(void*) [w95thred.c:f48eab99cc33d79d1ad62211c1f8d9d9c1cb6727 : 139 + 0xd]
20  ucrtbase.dll!thread_start<unsigned int (__cdecl*)(void *),1> + 0x42

(In reply to Gabriele Svelto [:gsvelto] from comment #3)

The volume here is crazy! We're looking at up to ~10k affected users per day. Is there something we can do about this?

Seems to affect to Konika Minolta Universal PCL Driver, all Printers, Drivers from 2005 to newest Driver and OEM's

about:config "print.tab_modal.enabled = false" is still working for me.

simmilar to BUG 1704592, may be the same issue.

Comment 7

[Bob Owen (:bobowen)]

I've tried to reproduce this with various Konica Minolta drivers, but to no avail.

We don't get useful stacks in most of the crashes but looking in more detail at some where we do, it appears that it is nearly always the DocumentPropertiesW call for SupportsMonochrome that is clashing with both DeviceCapabilitiesW and CreateICW in other threads.

So, the plan from comment 5 looks good.
Hopefully if we only lock around the calls it won't cause too many delays for other better behaving drivers.
I would imagine there will be other drivers that have the same problem as well and are crashing in a similar fashion.

Comment 8

[Bob Owen (:bobowen)]

Attachment: Bug 1694104: Add a Mutex to nsPrinterWin and lock around most driver calls. r=jfkthame!

This also removes the fix from bug 1677388, which worked in some circumstances,
but is effectively superseded by this one. Removing that fix also reduces
potential contention for copying the default DEVMODE.

Comment 9

[Bob Owen (:bobowen)]

https://treeherder.mozilla.org/jobs?repo=try&revision=211526c75cd5282aa908f3c5757e31d774eb7669

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a17f213bf4e
Add a Mutex to nsPrinterWin and lock around most driver calls. r=jfkthame

Comment 12

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/7a17f213bf4e

Comment 13

[Bob Owen (:bobowen)]

I've put esr78 as unaffected, because while we do see crashes in these drivers, they are in the main thread because esr78 doesn't have the new print UI changes.
So these are probably unrelated printer device crashes.

Comment 14

[Bob Owen (:bobowen)]

Comment on attachment 9216853 [details]
Bug 1694104: Add a Mutex to nsPrinterWin and lock around most driver calls. r=jfkthame!

Beta/Release Uplift Approval Request

• User impact if declined: Large number of browser crashes when printing with these printers and probably other printers whose drivers are not thread safe.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Putting medium, because while the changes are fairly straight forward they are not trivial and involve locking.
• String changes made/needed: None

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9216853 [details]
Bug 1694104: Add a Mutex to nsPrinterWin and lock around most driver calls. r=jfkthame!

Worth a shot, thanks. Approved for 89.0b12.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/596dbafbf237