Request for review- RFE 8005716

BILL PITTORE bill.pittore at oracle.com
Thu Mar 7 16:36:59 UTC 2013


I updated the code based on the feedback. To fix the windows symbol name 
issue that Dean brought up I added a platform specific function, 
buildJniFunctionName. In windows it will properly convert _JNI_OnLoad at 8 
to _JNI_OnLoad_libname at 8 as well as handle JNI_OnLoad symbol name.

Fixed copyright headers as well.

Tested on linux and windows

Webrevs are here:

http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.01/
http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/

bill

On 3/6/2013 8:47 AM, Alan Bateman wrote:
> On 05/03/2013 23:05, bill.pittore at oracle.com wrote:
>> This request is tied to bugid 8005716 that deals with adding support 
>> for statically linked JNI libraries.
>>
>> The JEP is here: http://openjdk.java.net/jeps/178
>>
>> The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716
>>
>> The webrevs are here:
>>
>> http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/
>> http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/
>>
>> The main piece of functionality is to check for the presence of 
>> JNI_OnLoad_libname to determine if the library specified by 'libname' 
>> has been statically linked into the VM. If the symbol is found, it is 
>> assumed that the library is linked in and will not be dynamically 
>> loaded.
> Overall I think this is quite good and follows the proposal in the 
> JEP. I don't see anything obvious wrong with the logic and everything 
> should just work for shared libraries as it does today. I assume 
> you'll run all the appropriate tests.
>
> I see Dean's suggestion to add a JVM function to allow for a lookup 
> table when the symbol information is available, If you do that then 
> you'll need to get the hotspot changes in first. Alternatively, change 
> what you have later once the hotspot changes are in.Just on the
>
> As findBuiltJniFunction can locate a built-in or a JNI_OnLoad/OnUnload 
> in a library then the function name is probably not quite right 
> (shouldn't have "Builtin" in the name).
>
> A minor nit in _findBuiltinLib is that if the OOME path should call 
> JNU_ReleaseStringPlatformChars before returning.
>
> There are a few commented out statements in jni_util_md.c that I 
> assume will be removed. Also you might want to check the indentation 
> in both getProcessHandle implementations as they look like 2-spaces 
> whereas the libs uses 4 (although this may be mute if you use a JVM 
> function).
>
> Otherwise I think this is good and I can sponsor the jdk part to this 
> and help get it into jdk8/tl.
>
> -Alan
>






More information about the core-libs-dev mailing list