<AWT Dev> [9] Review request for 7033533: realSync() doesn't work with Xfce

Anthony Petrov anthony.petrov at oracle.com
Fri Jan 24 03:35:26 PST 2014 

Hi Oleg,

Thanks for the update. The new version of the fix looks good to me. I'm 
wondering if we should preserve the workaround at original lines 
2430-2442 in XToolkit.java for added safety?

--
best regards,
Anthony

On 1/22/2014 8:31 PM, Oleg Pekhovskiy wrote:
> Hi Team,
>
>
> There was a long break after last comment and now I'm able to continue
> discussion.
> Please review the second version of fix:
> http://cr.openjdk.java.net/~bagiras/7033533.2/
>
> I investigated the order of different events and found that
> ConfigureNotify event goes through event queues (X Server and Xlib) the
> same way as SelectionNotify.
> Moreover it works with and without window manager. Please look at the
> native test and its log file attached to the issue in JIRA.
> Thus it could unify the way XToolkit.syncNativeQueue() marks the block
> of not handled events.
>
> There are several XLib functions that make X server generate
> ConfigureNotify event:
> XConfigureWindow(), XLowerWindow(), XRaiseWindow(), XRestackWindows(),
> XMoveWindow(), XResizeWindow(), XMoveResizeWindow(), XMapRaised(),
> XSetWindowBorderWidth().
> Invisible XAWTRootWindow could be used as a target for ConfigureNotify
> events.
> None of mentioned above functions are used for this window and it never
> becomes visible.
> These functions generate only one event (ConfigureNotify), contrary to
> XConvertSelection function which generates two (PropertyNotify,
> SelectionNotify).
>
> I chose XMoveWindow(), moving XAWTRootWindow back and forth by 1 pixel
> to avoid system optimization.
>
>
> Thanks,
> Oleg
>
> On 10/08/2013 05:09 PM, Anthony Petrov wrote:
>> Hi Artem,
>>
>> I'm not sure the testing results below are positive since in his
>> previous message Yuri wrote:
>>
>>> However some failures are suspicious. Try on Ubuntu with Unity.
>>
>> when testing on officially supported systems/window managers (see the
>> quote below).
>>
>> Also, Leonid and I have concerns regarding reliability of the current
>> version of the fix, and suggest to implement a solution that would try
>> executing the current code  of the realSync() first, and only if it
>> fails fall back to the workaround proposed by Oleg.
>>
>>
>> What is your opinion on this?
>>
>>
>> --
>> best regards,
>> Anthony
>>
>>
>> -------- Original Message --------
>> Subject: Re: <AWT Dev> [8] Review request for 7033533: realSync()
>> doesn't work with Xfce
>> Date: Fri, 04 Oct 2013 18:52:32 +0400
>> From: Yuri Nesterenko <yuri.nesterenko at oracle.com>
>> To: Leonid Romanov <leonid.romanov at oracle.com>,  Anthony Petrov
>> <anthony.petrov at oracle.com>
>> CC: Awt-Dev <awt-dev at openjdk.java.net>
>>
>> OK, Ubuntu 13.10 (final beta) with Mir instead of X11
>> looks surprisingly good. It fails in couple of places
>> with exceptions like
>> "sun.awt.X11.XException: Cannot write XdndAware property"
>> bug overall the patched build run the awt regression suite
>> quite OK.
>>
>> Thanks,
>> -yan
>>
>> On 10/04/2013 03:52 PM, Yuri Nesterenko wrote:
>>> So far, I've run some 300+ awt tests with patched build
>>> on Solaris 11 sparcv9 and Ubuntu 13.04 Unity.
>>>
>>> Overall passrate is regular.
>>> Most of the failing tests using realSync do fail rather
>>> similarly with patched build and b110.
>>>
>>> However some failures are suspicious. Try on Ubuntu with Unity.
>>>
>>> test/java/awt/Toolkit/ScreenInsetsTest/ScreenInsetsTest.java
>>> consistently fails some 6 out of 20 runs with patched build but
>>> never in the promoted b110. It may be test issue though, somehow.
>>>
>>> event/MouseEvent/SpuriousExitEnter/SpuriousExitEnter_3.java
>>> fails in half of the runs with "Incorrect event number" but
>>> always pass with b110 (4 or 5 runs).
>>> Event number! It may be superficial resemblance but --
>>>
>>> Choice/ChoiceMouseWheelTest/ChoiceMouseWheelTest.java --
>>> coincidence here: Oleg, perhaps you should use
>>> @library ../../regtesthelpers and
>>> @build Util
>>> in these regtests: import test.java.awt.regtesthelpers.Util fails.
>>>
>>> Thanks,
>>> -yan
>>>
>>> On 10/01/2013 04:27 PM, Yuri Nesterenko wrote:
>>>> Sure we can say that Xfce is not complying -- it is not officially
>>>> supported by JDK -- neither is LXDE etc. -- but saying that gets
>>>> us nowhere.
>>>>
>>>> As to testing, could you suggest a platform selection? I'm afraid
>>>> we'll not be able to test Xsun properly but Xorg with Gnome on
>>>> Linux and Solaris, Unity and Xfce4 --
>>>> all that we can do by the weekend.
>>>>
>>>> Thanks,
>>>> -yan
>>>>
>>>> On 10/01/2013 04:01 PM, Leonid Romanov wrote:
>>>>> By the way, I was reading Inter-Client Communication Conventions
>>>>> Manual
>>>>> so I could better understand the fix, and found the following:
>>>>>
>>>>> "4.3. Communication with the Window Manager by Means of Selections
>>>>>
>>>>> For each screen they manage, window managers will acquire ownership
>>>>> of a
>>>>> selection named WM_Sn, where n is the screen number, as described in
>>>>> section 1.2.6. Window managers should comply with the conventions for
>>>>> "Manager Selections" described in section 2.8. The intent is for
>>>>> clients
>>>>> to be able to request a variety of information or services by issuing
>>>>> conversion requests on this selection."
>>>>>
>>>>> Considering this, can we say that Xfce is not complying to the spec?
>>>>>
>>>>> On 10/1/2013 15:29, Anthony Petrov wrote:
>>>>>> Hi Oleg,
>>>>>>
>>>>>> I second to Leonid: you should add a comment and explain why you
>>>>>> expect exactly 4 (or more) events to be processed. Preferably, you
>>>>>> should list each event to clearly understand this.
>>>>>>
>>>>>> A minor comment is, lines 2404 - 2407 should be moved to the nearest
>>>>>> try{} block at line 2409.
>>>>>>
>>>>>> A major concern is that I'm not sure the new solution is reliable in
>>>>>> all cases. Previously, we expected to get a notification from another
>>>>>> process (the WM). Now we process the notification ourselves and are
>>>>>> the owners of the selection. I don't know for sure, but suppose some
>>>>>> xlib implementations could optimize this scenario and process events
>>>>>> locally w/o even sending them to the X server. In which case there
>>>>>> wouldn't be any real synchronization of the native event queue.
>>>>>> That's
>>>>>> why communicating with another process was an important part of this
>>>>>> procedure.
>>>>>>
>>>>>> How about we try the original method first, and only if it fails,
>>>>>> then
>>>>>> try this workaround solution?
>>>>>>
>>>>>> Also, this is a very sensitive method because a lot of code relies on
>>>>>> it. I suggest running all automatic regression tests for AWT and
>>>>>> Swing
>>>>>> areas to make sure we don't introduce a regression with this fix.
>>>>>>
>>>>>> --
>>>>>> best regards,
>>>>>> Anthony
>>>>>>
>>>>>> On 09/26/2013 11:56 AM, Oleg Pekhovskiy wrote:
>>>>>>> Hi Leonid,
>>>>>>>
>>>>>>> I did it mostly logically, because my fix added one more service
>>>>>>> event
>>>>>>> (SelectionRequest), that should be excluded from the number of
>>>>>>> dispatched native events.
>>>>>>> IMHO, the previous number "2" should have been commented, but, that
>>>>>>> did
>>>>>>> not happen.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Oleg
>>>>>>>
>>>>>>> On 25.09.2013 18:11, Leonid Romanov wrote:
>>>>>>>> Hi,
>>>>>>>> I'm not an expert in X11 programming, so I can't comment about the
>>>>>>>> fix
>>>>>>>> in general, but I think the line 2436, "return getEventNumber() -
>>>>>>>> event_number > 3", really asks for a comment.
>>>>>>>>
>>>>>>>> On 9/25/2013 16:38, Oleg Pekhovskiy wrote:
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> please review the fix
>>>>>>>>>
>>>>>>>>> for
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-7033533
>>>>>>>>>
>>>>>>>>> Previous implementation of XToolkit.syncNativeQueue() relied upon
>>>>>>>>> WM_S0 atom existence and that it was owned by current window
>>>>>>>>> manager.
>>>>>>>>> But several WMs (like XFCE and LXDE) don't send SelectionNotify
>>>>>>>>> event
>>>>>>>>> to the client on XConvertSelection() for that atom. That led
>>>>>>>>> XToolkit.syncNativeQueue() to hang until TimeOutException.
>>>>>>>>>
>>>>>>>>> I decided to keep XConvertSelection() usage, but make root toolkit
>>>>>>>>> window as an owner for selection atom (with another name), and
>>>>>>>>> handle
>>>>>>>>> SelectionRequest event from X Server, sending SelectionNotify in
>>>>>>>>> response (as window manager is supposed to do).
>>>>>>>>>
>>>>>>>>> Tested on both XFCE and LXDE.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Oleg
>>>>>>>>
>>>>>
>>>>
>>>
>>
>>
>>
>

More information about the awt-dev mailing list