[OpenJDK 2D-Dev] RFR: 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames()

[Prasanta Sadhukhan]

On Fri, 5 Mar 2021 01:27:47 GMT, Sergey Bylokhov <serb at 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.
>
> src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp line 136:
> 
>> 134: 
>> 135:     try {
>> 136:         ::EnumPrinters(flags, NULL, 4, NULL,
> 
> Don't we need to check that this method call succeeds? Probably it is a crash reason when the EnumPrinters fails but we use cbNeeded anyway for array allocation?

I guess since we are changing this method anyway, can we use PRINTER_ENUM_CONNECTIONS flag instead of hardcoded 4 so that it is more informative!!

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

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