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