On 05/06/2016 04:33 PM, Volker Simonis wrote:
Hi Phil,
Thanks for looking at this problem.
On Saturday, May 7, 2016, Phil Race <philip.r...@oracle.com
<mailto:philip.r...@oracle.com>> 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/
<http://cr.openjdk.java.net/%7Eprr/8156020/>
I don't think this will work correctly on AIX (although it may
build) because AIX only has a crippled /proc file system. And it won't
work on any other platform without /proc file system (it is actually
even less "POSIX" than RTLD_NOLOAD :)
Hmm. In internal discussions /proc was proposed precisely because
it was more widely (may be universally available).
How can you be sure that isLoadedLib() is really returning false
because the library is not loaded and not because the corresponding
file in the proc file system wasn't found?
This is definitely still early days but it was claimed to be more reliable
although we filed a follow-on bug because it did not seem like the kind
of thing we want to do. Even finding something that works reliably on Linux
is not final. It may be that this is something that needs to be tweaked for
each platform port,
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
It is OK for me if we fix this in client first as this seems to be the
easiest solution process-wise. But are you sure v6 gives you the right
answers on all you platforms and not just false positives as did the
v5 version?
Heck no :-). I know there was promising testing but it's early.
I am not sure this has been tried at all on Solaris either.
If you will check in v6 now I can fix it on AIX on Monday if that
should still give build problems.
Well it looks like v6 is about to finish up in JPRT so either is an option.
Your comments about /proc make me more inclined to go with your code.
So now I know more and have compared these how about we do as you
originally proposed. You push it to dev and Monday I will sync it into
client
and we'll take it from there.
But maybe after the build fixes we should go for a more general
solution and define platform-specific "isLibLoaded()" functions?
Yes, this is a work-in-progress.
-phil.
Regards,
Volker
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/
<http://cr.openjdk.java.net/%7Esimonis/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
