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. ??
Although we may have not used RTLD_NOW with static builds (does this
discussions re symbol resolution even make sense with a static build?)
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.
So it is possible for libjava to be opened without RTLD_NOW. I think
the proposed change should work correctly in that case. The dladdr
should find the current object, which we'll open by name (rather than
using NULL, but that should be okay), using RTLD_NOW, forcing
resolution of all symbols. I'm not sure there can be any relevant
unresolved symbols on this path, but then, as said before, I don't
understand the point of this workaround.
I’m reluctant to remove this completely without a better understanding of what
it is there for and why
it is no longer needed, if indeed that’s the case. The lack of problems when
running tests is suggestive
but not definitive.
I'd be happier knowing the exact details here as well, but if the new code
doesn't implement the workaround then it's no better than completely removing
it. And as we seem to have been always already using the new code then …
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.
Perhaps the old per-file flag setting can be restored. By all means file
a RFE but I fear there is noone who would know the details of this any
more. :(
David
