<Swing Dev> <AWT Dev> RFR(S): 8156020: 8145547 breaks AIX and and uses RTLD_NOLOAD incorrectly

[Sergey Bylokhov]

Hi.
I guess the version .5 was pushed because the .6 parse "/proc".

On 07.05.16 1:56, Phil Race wrote:
> Hi,
>
> I've confirmed that what was pushed was v5 and it should really have
> been v6.
> I can't say unequivocally that it would have built on AIX but
> 1) It does not use RTLD_NOLOAD anywhere
> 2) all calls to dlopen include RTLD_LAZY.
>
> Here is the delta - the patch that makes v5 into v6 and although
> it is not quite the same as yours it bears a striking resemblance
> http://cr.openjdk.java.net/~prr/8156020/
>
> JPRT did build your patch (including on our embedded builds), and
> I am doing the same - in progress - for the v5->v6 delta but I have a
> dilemma.
>
> Option 1) Apply the delta of v5 -> v6 to client to get us where we
> should be
> and you "workaround it" in AIX until it makes its way to dev
>
> Option 2) Apply one of these patches to dev and sync it back to client
> and clean up the mess later
>  2a) apply v5->v6
>  2b) apply Volker's patch and fix up the mess of the difference later.
>
> We are taking something of a risk in applying either to dev so I will
> need to do some kind of sanity checking
>
> Opinions ?
>
> -phil.
>
>
> On 05/05/2016 09:32 PM, Philip Race wrote:
>> Hi Volker,
>>
>> 1) adding awt-dev. Semyon did the review on swing but really it should
>> always
>> (and mainly!) have been awt.
>>
>> 2) Yes, this ought to be pushed to 9-client, specifically not 9-dev.
>> Assuming it goes to 9-dev we may need to deal with conflicts.
>> Also if it causes any kind of problem with 9-dev I would not want to pile
>> fix on fix, so it would probably just get anti-deltaed. Just a warning.
>>
>> 3) It strictly needs a JPRT run before pushing so someone will need to
>> do that.
>>
>> 4) This change definitely needs two reviewers.
>>
>> And we were discussing RTLD_NOLOAD is not Posix as that came up why it
>> was not
>> a solution in a cross-platform solution for determining whether libs were
>> already loaded but it was reported to not be able to detect some cases.
>> So I thought we had determined it was not a general solution.
>> Leaving aside why it is in there after that (something I will need to
>> check),
>> the lack of the other flag may explain why it was apparently "not
>> working".
>>
>> So one interesting thing is it appears to me that I thought we pushed
>> the .v6 webrev - the one I thought we (or I) approved since it was the
>> latest
>> obviously
>> http://mail.openjdk.java.net/pipermail/swing-dev/2016-April/005684.html
>>
>> but this looks like the v5 webrev was pushed :
>> http://mail.openjdk.java.net/pipermail/swing-dev/2016-April/005678.html
>>
>> All of this "detection" code was the main issue at that juncture.
>> So I would like some time to disentangle that before anything is changed.
>>
>> -phil
>>
>> On 5/4/16, 11:32 AM, Volker Simonis wrote:
>>> Hi,
>>>
>>> can somebody please review this small change which fixes the AIX build
>>> after change 8145547, but also fixes an incorrect usage pattern of
>>> RTLD_NOLOAD in 8145547:
>>>
>>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8156020/
>>> https://bugs.openjdk.java.net/browse/JDK-8156020
>>>
>>> Here are the details from the bug report:
>>>
>>> Change 8145547 uses the RTLD_NOLOAD flag when calling dlopen to probe
>>> the availability of the GTK libraries.
>>>
>>> But unfortunately RTLD_NOLOAD is not Posix and for example not
>>> available on AIX and BSD.
>>>
>>> I also found out, that the implementation of 8145547 contains a bug.
>>> It uses RTLD_NOLOAD in an incorrect way. The man page for dlopen
>>> clearly states that one of the two flags RTLD_LAZY or RTLD_NOW has to
>>> be included in the flags. But the current implementation uses
>>> RTLD_NOLOAD as single flag. Therefor the call to dlopen() currently
>>> always returns NULL, no difference if the corresponding library has
>>> been loaded already or not.
>>>
>>> The bug report also contains a small C program which can be used to
>>> reproduce the problem.
>>>
>>> The fix is to only use RTLD_NOLOAD if it is defined. The change
>>> removes the 'flags' argument from the various check() functions and
>>> replaces it with a boolean 'load' argument. It indicates if the check
>>> functions should just look for a previously loaded version of the GTK
>>> libraries (i.e. if 'load' == false) or if it should additionally try
>>> to load the libraries if that hasn't been done before (i.e. if 'load'
>>> == true).
>>>
>>> I hope I haven't changed the previous program semantics with my
>>> change. At least I couldn't see any difference :)
>>>
>>> I've built and smoke tested on Linux/Solaris and AIX with various
>>> combinations for jdk.gtk.version,
>>> -Dswing.defaultlaf=com.sun.java.swing.plaf.gtk.GTKLookAndFeel and
>>> FileDialog implementations.
>>>
>>> I'd like to push this directly to jdk9-dev to fix the AIX build as
>>> fast as possible. Would that be OK?
>>>
>>> Thank you and best regards,
>>> Volker
>


-- 
Best regards, Sergey.