Re: [OpenJDK 2D-Dev] RFR: 8262446: DragAndDrop hangs on Windows

2021-03-04 Thread Ichiroh Takiguchi
On Thu, 4 Mar 2021 10:36:56 GMT, Dmitry Markov  wrote:

> The IME functions and the DND operation must be executed on the toolkit 
> thread. If the DND operation is in progress, the IME API is invoked via 
> SendMessage() call inside InvokeInputMethodFunction() to avoid a hang. The 
> flag isInDoDragDropLoop indicates whether the DND takes place or not. The 
> flag works properly if the DND is performed between two Java windows. However 
> if anything is dragged from native app, (e.g. Windows FileExplorer) to Java 
> the flag is NOT set. That’s the root cause of the hang.
> 
> Fix:
> Introduce a new flag to indicate DND operation between Java and native app. 
> 
> Testing:
> mach5 green

@dmarkov20 
I'd like to confirm this issue was not fixed by 
[JDK-8261231](https://bugs.openjdk.java.net/browse/JDK-8261231) #2448 ?

-

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


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

2021-03-04 Thread Prasanta Sadhukhan
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()

2021-03-04 Thread Prasanta Sadhukhan
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: 8262446: DragAndDrop hangs on Windows

2021-03-04 Thread Sergey Bylokhov
On Thu, 4 Mar 2021 19:38:34 GMT, Alexey Ivanov  wrote:

>> The IME functions and the DND operation must be executed on the toolkit 
>> thread. If the DND operation is in progress, the IME API is invoked via 
>> SendMessage() call inside InvokeInputMethodFunction() to avoid a hang. The 
>> flag isInDoDragDropLoop indicates whether the DND takes place or not. The 
>> flag works properly if the DND is performed between two Java windows. 
>> However if anything is dragged from native app, (e.g. Windows FileExplorer) 
>> to Java the flag is NOT set. That’s the root cause of the hang.
>> 
>> Fix:
>> Introduce a new flag to indicate DND operation between Java and native app. 
>> 
>> Testing:
>> mach5 green
>
> Marked as reviewed by aivanov (Reviewer).

Why we cannot reuse the old flag? "isInDoDragDropLoop"? I think the 
Robot.waitForIdle() will hang if isInDoDragDropLoop is not set to true while 
dragging something from the native app.

-

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


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

2021-03-04 Thread Sergey Bylokhov
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()

2021-03-04 Thread Phil Race
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()

2021-03-04 Thread Alexey Ivanov
**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


Re: [OpenJDK 2D-Dev] RFR: 8262446: DragAndDrop hangs on Windows

2021-03-04 Thread Alexey Ivanov
On Thu, 4 Mar 2021 10:36:56 GMT, Dmitry Markov  wrote:

> The IME functions and the DND operation must be executed on the toolkit 
> thread. If the DND operation is in progress, the IME API is invoked via 
> SendMessage() call inside InvokeInputMethodFunction() to avoid a hang. The 
> flag isInDoDragDropLoop indicates whether the DND takes place or not. The 
> flag works properly if the DND is performed between two Java windows. However 
> if anything is dragged from native app, (e.g. Windows FileExplorer) to Java 
> the flag is NOT set. That’s the root cause of the hang.
> 
> Fix:
> Introduce a new flag to indicate DND operation between Java and native app. 
> 
> Testing:
> mach5 green

Marked as reviewed by aivanov (Reviewer).

-

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


Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]

2021-03-04 Thread Vladimir Kempik
On Thu, 4 Mar 2021 17:36:22 GMT, Alan Hayward 
 wrote:

> I was building this PR on a new machine, and I now get the following error:
> 
> > /Users/alahay01/java/gerrit_jdk/src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_MidiUtils.c:258:31:
> >  error: cast to smaller integer type 'MIDIClientRef' (aka 'unsigned int') 
> > from 'void *' [-Werror,-Wvoid-pointer-to-int-cast]
> > static MIDIClientRef client = (MIDIClientRef) NULL;
> > ^~~~
> > /Users/alahay01/java/gerrit_jdk/src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_MidiUtils.c:259:29:
> >  error: cast to smaller integer type 'MIDIPortRef' (aka 'unsigned int') 
> > from 'void *' [-Werror,-Wvoid-pointer-to-int-cast]
> > static MIDIPortRef inPort = (MIDIPortRef) NULL;
> > ^~
> > /Users/alahay01/java/gerrit_jdk/src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_MidiUtils.c:260:30:
> >  error: cast to smaller integer type 'MIDIPortRef' (aka 'unsigned int') 
> > from 'void *' [-Werror,-Wvoid-pointer-to-int-cast]
> > static MIDIPortRef outPort = (MIDIPortRef) NULL;
> > ^~
> > /Users/alahay01/java/gerrit_jdk/src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_MidiUtils.c:466:32:
> >  error: cast to smaller integer type 'MIDIEndpointRef' (aka 'unsigned int') 
> > from 'void *' [-Werror,-Wvoid-pointer-to-int-cast]
> > MIDIEndpointRef endpoint = (MIDIEndpointRef) NULL;
> > ^~
> > 4 errors generated.
> 
> As far as I can tell the only difference between the two systems is the xcode 
> version:
> 
> New system (failing)
> % xcodebuild -version
> Xcode 12.5
> Build version 12E5244e
> 
> Old system (working)
> % xcodebuild -version
> Xcode 12.4
> Build version 12D4e
> 
> Looks like the newer version of Xcode is being a little stricter with casting?
> Replacing the NULL with 0 seems to fix the issue.

Hello
there is one issue with the info you provided, it's from Xcode12.5 beta.
And beta license agreement forbids sharing output of beta version of compiler
So we can't say we have issue with newer xcode beta until that beta went public 
& released.
Fixing issues you found now will mean someone have violated xcode beta license 
agreement and made these issues public.

-

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


Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]

2021-03-04 Thread Alan Hayward
On Thu, 4 Mar 2021 15:27:25 GMT, Gerard Ziemski  wrote:

>>> A list of the bugs that our internal testing revealed so far:
>> 
>> Are any of these blockers for integration? Some of them are to do with 
>> things like features that aren't yet supported, and we can't fix what we 
>> can't see.
>
>> > A list of the bugs that our internal testing revealed so far:
>> 
>> Are any of these blockers for integration? Some of them are to do with 
>> things like features that aren't yet supported, and we can't fix what we 
>> can't see.
> 
> I don't personally think any of these issues are blockers. It's a great 
> effort as it is and very much appreciated. Anything else can be fixed as a 
> followup.
> 
> There might be some legal requirements (i.e. JCK) that I'm not in position to 
> comment on, however, so someone else might need to chime in here.

I was building this PR on a new machine, and I now get the following error:

> /Users/alahay01/java/gerrit_jdk/src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_MidiUtils.c:258:31:
>  error: cast to smaller integer type 'MIDIClientRef' (aka 'unsigned int') 
> from 'void *' [-Werror,-Wvoid-pointer-to-int-cast]
> static MIDIClientRef client = (MIDIClientRef) NULL;
>   ^~~~
> /Users/alahay01/java/gerrit_jdk/src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_MidiUtils.c:259:29:
>  error: cast to smaller integer type 'MIDIPortRef' (aka 'unsigned int') from 
> 'void *' [-Werror,-Wvoid-pointer-to-int-cast]
> static MIDIPortRef inPort = (MIDIPortRef) NULL;
> ^~
> /Users/alahay01/java/gerrit_jdk/src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_MidiUtils.c:260:30:
>  error: cast to smaller integer type 'MIDIPortRef' (aka 'unsigned int') from 
> 'void *' [-Werror,-Wvoid-pointer-to-int-cast]
> static MIDIPortRef outPort = (MIDIPortRef) NULL;
>  ^~
> /Users/alahay01/java/gerrit_jdk/src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_MidiUtils.c:466:32:
>  error: cast to smaller integer type 'MIDIEndpointRef' (aka 'unsigned int') 
> from 'void *' [-Werror,-Wvoid-pointer-to-int-cast]
> MIDIEndpointRef endpoint = (MIDIEndpointRef) NULL;
>^~
> 4 errors generated.

As far as I can tell the only difference between the two systems is the xcode 
version:

New system (failing)
% xcodebuild -version
Xcode 12.5
Build version 12E5244e

Old system (working)
% xcodebuild -version
Xcode 12.4
Build version 12D4e

Looks like the newer version of Xcode is being a little stricter with casting?
Replacing the NULL with 0 seems to fix the issue.

-

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


Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]

2021-03-04 Thread Gerard Ziemski
On Wed, 3 Mar 2021 17:46:41 GMT, Andrew Haley  wrote:

> > A list of the bugs that our internal testing revealed so far:
> 
> Are any of these blockers for integration? Some of them are to do with things 
> like features that aren't yet supported, and we can't fix what we can't see.

I don't personally think any of these issues are blockers. It's a great effort 
as it is and very much appreciated. Anything else can be fixed as a followup.

There might be some legal requirements (i.e. JCK) that I'm not in position to 
comment on, however, so someone else might need to chime in here.

-

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


[OpenJDK 2D-Dev] RFR: 8262446: DragAndDrop hangs on Windows

2021-03-04 Thread Dmitry Markov
The IME functions and the DND operation must be executed on the toolkit thread. 
If the DND operation is in progress, the IME API is invoked via SendMessage() 
call inside InvokeInputMethodFunction() to avoid a hang. The flag 
isInDoDragDropLoop indicates whether the DND takes place or not. The flag works 
properly if the DND is performed between two Java windows. However if anything 
is dragged from native app, (e.g. Windows FileExplorer) to Java the flag is NOT 
set. That’s the root cause of the hang.

Fix:
Introduce a new flag to indicate DND operation between Java and native app. 

Testing:
mach5 green

-

Commit messages:
 - 8262446: DragAndDrop hangs on Windows

Changes: https://git.openjdk.java.net/jdk/pull/2825/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2825=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8262446
  Stats: 10 lines in 3 files changed: 9 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2825.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2825/head:pull/2825

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