RFR: 8280468: Crashes in getConfigColormap, getConfigVisualId, XVisualIDFromVisual on Linux [v3]

[Maxim Kartashev]

On Sat, 9 Apr 2022 06:02:13 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>>> I don't understand why this was extracted out. It isn't called anywhere else and again makes the diff harder to understand.
>> 
>> This AWT lock should be held for a very small portion of the original method. Initially, I left the code guarded by it in place and it looked weird and out of place. I understand that this is a judgement call, though.
>> 
>> As for the diffs, they are not very significant (essentially, 4 lines of code were re-grouped and moved around) and we only have to look at them here for a few days. The (refactored) code itself will be looked at by (presumably) a greater number of people over a far greater period of time. Less of a judgement call.
>
>>This is just a named copy of lines 67 to 75. Since now this is the only piece of code that needs to work holding the AWT lock, I thought it best to isolate it.
> As I understand, the purpose of the piece is to find the "new" X11GraphicsDevice with the screen number same to the given "old" GraphicsConfiguration.
> 
> I am not sure that we can have some "old" and "new" devices which have the same screen id.
>  * If the list of devices was not changed then the "screenid" for old device is the index of that "old" device in the device array. So the new and old devices are the same.
>  * If the list of devices was changed, then the "screenid" for the old device was invalidated, and it is again an index of that "old" device in the device array. Also I do not think that we can change the "requested device" in this method, The purpose of this method is to request the best "GC" from the device passed as a parameter(via another GC).
> 
> Please take a look to the fix for JDK-6804747, initially that method get the screen index as a parameter, and then replaced by the GC. The code of the method did not changed but the index just requested from the GC.
> 
> Probably the code is just buggies?

@mrserb I agree with your logic, but fixing this now would certainly be not a semantically null change and would complicate the fix that has already been under review for over two months. I'd be happy to file a bug for this separate issue.

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

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