On Thu, 4 Mar 2021 21:37:33 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:

> **Root cause:**
> `getPrinterNames()` has two calls to 
> [`::EnumPrinters`](https://docs.microsoft.com/en-us/windows/win32/printdocs/enumprinters).
>  The first call is to get the required buffer size to contain the structures 
> and any strings. The second call is to get the list of printers.
> 
> Yet the list of printers or the names of printers can change between the two 
> calls. If it happens, the second call to `EnumPrinters` fails but it is not 
> checked.
> 
> I couldn't reproduce the crash myself. However, I reproduced the failure in 
> the second call to `::EnumPrinters` by adding `::Sleep(500)` and by renaming 
> the printers so that the data doesn't fit into the allocated buffer:
> 
> 1: bResult: 0, cbNeeded: 504, cReturned: 0
> 2: bResult: 0, cbNeeded: 512, cReturned: 0
>     !!! error
> 
> During my testing, `cReturned` has always been zero whenever `EnumPrinters` 
> fails.
> 
> The crash dumps show that `cReturned` is non-zero but the `pPrinterEnum` 
> buffer doesn't contain valid data. Reading `info4->pPrinterName` at the line
> utf_str = JNU_NewStringPlatform(env, info4->pPrinterName);
> raises Access Violation exception.
> 
> **The fix:**
> Check the return value of `EnumPrinters` and allow for 5 retries. If the 
> function still returns failure, make sure `cReturned` is set to zero.
> 
> I'm going to close 
> [JDK-6996051](https://bugs.openjdk.java.net/browse/JDK-6996051) and 
> [JDK-8182683](https://bugs.openjdk.java.net/browse/JDK-8182683) reported 
> previously about the same crash as duplicate of the current 
> [JDK-8262829](https://bugs.openjdk.java.net/browse/JDK-8262829).
> 
> **Testing:**
> I used 
> [`RemotePrinterStatusRefresh.java`](https://github.com/openjdk/jdk/blob/master/test/jdk/java/awt/print/RemotePrinterStatusRefresh/RemotePrinterStatusRefresh.java)
>  for testing, it shows the list of currently available printers. In the 
> background I ran `PrinterRename.ps1` PowerShell script which remains a 
> printer, the script is attached to the JBS issue.
> 
> Without the fix, the list of available printers was empty occasionally 
> because `EnumPrinters` returned failure and set `cReturned` to zero. With the 
> fix, the list of printers is always filled.

I guess this is OK and yes we should have been checking this.
Not sure we really got to the bottom of the real world problem because I'd 
expect the 2nd call just milliseconds after the first. But it could be that 
this happens during some system updates and we get one printer reconfigured 
message followed by another ..

-------------

Marked as reviewed by prr (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2836

Reply via email to