Re: [OpenJDK 2D-Dev] RFR: 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames()
On Thu, 4 Mar 2021 21:37:33 GMT, Alexey Ivanov 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. Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2836
Re: [OpenJDK 2D-Dev] RFR: 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames()
On Thu, 4 Mar 2021 21:37:33 GMT, Alexey Ivanov 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. Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2836
Re: [OpenJDK 2D-Dev] RFR: 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames()
On Thu, 4 Mar 2021 22:09:55 GMT, Phil Race wrote: > 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 .. Most of the time, the crash happens when a client reconnects to their active session where a JVM is already running. On reconnect, the list of printers is synced with the local printers on the client. In such a scenario, other software could also update its list of printers as well as repaint its UI. If the system is under high load, it's not impossible to have a long enough pause between the calls. - PR: https://git.openjdk.java.net/jdk/pull/2836
Re: [OpenJDK 2D-Dev] RFR: 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames()
On Fri, 5 Mar 2021 03:49:22 GMT, Prasanta Sadhukhan wrote: >> 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!! > > ok, it is for flags and not for level. please ignore > > > 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? `EnumPrinters` always returns failure here because zero-sized buffer is too small to contain any data. - PR: https://git.openjdk.java.net/jdk/pull/2836
Re: [OpenJDK 2D-Dev] RFR: 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames()
On Fri, 5 Mar 2021 03:42:29 GMT, Prasanta Sadhukhan
wrote:
>> 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!!
ok, it is for flags and not for level. please ignore
-
PR: https://git.openjdk.java.net/jdk/pull/2836
Re: [OpenJDK 2D-Dev] RFR: 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames()
On Fri, 5 Mar 2021 01:27:47 GMT, Sergey Bylokhov 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
Re: [OpenJDK 2D-Dev] RFR: 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames()
On Thu, 4 Mar 2021 21:37:33 GMT, Alexey Ivanov 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?
-
PR: https://git.openjdk.java.net/jdk/pull/2836
Re: [OpenJDK 2D-Dev] RFR: 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames()
On Thu, 4 Mar 2021 21:37:33 GMT, Alexey Ivanov 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
[OpenJDK 2D-Dev] RFR: 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames()
**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. - Commit messages: - 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames() Changes: https://git.openjdk.java.net/jdk/pull/2836/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2836=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8262829 Stats: 20 lines in 1 file changed: 13 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/2836.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2836/head:pull/2836 PR: https://git.openjdk.java.net/jdk/pull/2836
