On 3/10/2018 6:51 AM, Kim Barrett wrote:
On Oct 1, 2018, at 7:10 PM, David Holmes <david.hol...@oracle.com> wrote:
On 2/10/2018 4:35 AM, Kim Barrett wrote:
On Oct 1, 2018, at 2:27 AM, David Holmes <david.hol...@oracle.com> wrote:
On 1/10/2018 1:21 PM, Kim Barrett wrote:
The current library may have been previously only dlopen’ed with RTLD_LAZY.
Reopening with RTLD_NOW
forces resolution of all undefined symbols in that shared object, which appears
to be the purpose of the
deprecated function.
The current library is libjvm and it's already opened in the appropriate way:
libjvm = dlopen(jvmpath, RTLD_NOW + RTLD_GLOBAL);
and it seems to have always been opened this way. So I can't see how this can
fix the problem the workaround purports to fix??
http://hg.openjdk.java.net/macosx-port/macosx-port/hotspot/rev/be94a5de4d32#l54.758
The current version of LoadJavaVM (for Mac) contains:
#ifndef STATIC_BUILD
libjvm = dlopen(jvmpath, RTLD_NOW + RTLD_GLOBAL);
#else
libjvm = dlopen(NULL, RTLD_FIRST);
#endif
where STATIC_BUILD is controlled by the --enable-static-builds
configure option. The --enable-static-builds option was added in JDK 9
by 8136556: Add the ability to perform static builds of MacOSX x64
binaries.
Which "current version" is this? It is not what is in jdk/jdk repo, which has
no STATIC_BUILD support in this area. ??
I think the MacOSX version of LoadJavaVM is here:
./java.base/macosx/native/libjli/java_md_macosx.m
(Note that this appears to be an Obj-C source file.)
(And no, I have no idea what build magic goes on to make this work.)
This has the STATIC_BUILD stuff.
Hmmm okay. But I can't find that file in the original mac port, or in
7u at all. In 7u we have
jdk/src/macosx/bin/java_md_macosx.c
with
libjvm = dlopen(jvmpath, RTLD_NOW + RTLD_GLOBAL);
and that was created in 2012 when the launcher was refactored under
JDK-7124089.
The archaeology is getting too hard. :(
There are also definitions for other platforms.
./java.base/windows/native/libjli/java_md.c // Windows
./java.base/unix/native/libjli/java_md_solinux.c // Solaris/Linux/AIX
(Note that java_md_solinux.c is excluded for macosx by the build
system; see LIBJLI_EXCLUDE_FILES in CoreLibraries.gmk.)
Although we may have not used RTLD_NOW with static builds (does this
discussions re symbol resolution even make sense with a static build?)
I think symbol resolution can make sense with a static build, if there
are other libraries that are not statically linked. I don't know
whether that's the case here. I'm going to try doing a static build
without this dlopen code and see what happens.
in some version of the JDK, my point is that we have been using RTLD_NOW for as
long as the workaround has been in place.
That strongly suggests to me that use of RTLD_NOW is not a solution to the
problem the workaround was introduced for.
I think my replacement code is equivalent to the deprecated code. I
think the sequence of events may have been something like:
(1) dlopen java lib without RTLD_NOW
(2) run into problems that led to workaround code
(3) run into further problems that led to dlopen java lib with RTLD_NOW
but didn't remove the now superfluous workaround (perhaps it was
forgotten?)
(4) merge
Perhaps. No way to know.
I thought this part of the change would be straight-forward. An
alternative would be to leave the old deprecated call in place,
locally disable the deprecation warning, and file an RFE for someone
more knowledgable in this area and with Mac development to look at.
Sorry for the obstructions on this but changing the code without knowing it
even implements the same workaround is just wrong to me. I strongly suspect the
code is not needed at all.
I agree that for the non-STATIC_BUILD case the workaround is
superfluous. If a static build without any workaround code works then
I think the workaround code should just be removed. Is that okay with
you?
Yes but you will need to go through a full round of testing, not just
tier 1-3, before being reasonably confident the workaround is not
needed. I'd hate to see this break things "in the wild" just because we
don't have adequate test coverage.
Other opinions welcomed.
David
