[OpenJDK 2D-Dev] [9] RFR JDK-8162461: Hang due to JNI up-call made whilst holding JNI critical lock.

Philip Race philip.race at oracle.com
Thu Sep 8 21:26:00 UTC 2016


I think the v.1 will be better even if it is not showing up as an issue,
given that you seem to have a safe + simple fix to that part

So "+1" to "1"

-phil.

On 9/8/16, 8:09 AM, Jayathirth D V wrote:
> Hi Phil,
>
> More observations:
> All emit_message() calls come under macros defined in jerror.h like WARNXX and TRACEXX.
> I made changes in IJG library so that we call these WARNXX and TRACEXX macros forcefully in turn calling emit_message()(emit_message() also changed to call output_message() everytime).
>
> With the above change and without RELEASE/GET_ARRAYS change in sun_jpeg_output_message() also JVM is not throwing any error. It means when we enter sun_jpeg_output_message()  we don't have any active JNI critical lock.
>
> We can actually use http://cr.openjdk.java.net/~jdv/8162461/webrev.00/ without RELEASE/GET_ARRAYS change in sun_jpeg_output_message()  or we can use http://cr.openjdk.java.net/~jdv/8162461/webrev.01/ having RELEASE/GET_ARRAYS change in sun_jpeg_output_message()  for future proofing.
>
> Thanks,
> Jay
>
> -----Original Message-----
> From: Jayathirth D V
> Sent: Thursday, September 08, 2016 7:21 PM
> To: Philip Race; 2d-dev
> Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-8162461: Hang due to JNI up-call made whilst holding JNI critical lock.
>
> Hi Phil,
>
> Thanks for pointing me to  sun_jpeg_output_message().
>
> We need to make up calls from sun_jpeg_output_message()  as it is needed if user has added IIOReadWarningListener.
> In imageioJPEG.c we are overriding IJG output_message() of jerror.c with our own  sun_jpeg_output_message().
>
> Call from IJG library can reach output_message() through two functions in jerror.c :
> 	1) error_exit()
> 	2) emit_message()
>
> We are overriding error_exit() also with sun_jpeg_error_exit() where we are not calling output_message(), so sun_jpeg_output_message() can be reached only through emit_message() .
>
> emit_message() always takes j_common_ptr as argument and not j_compress_ptr or j_decompress_ptr. But I noticed that before calling emit_message() we are always type casting j_compress_ptr or j_decompress_ptr to j_common_ptr and passing it as argument to emit_message().
>
> Since cinfo->  is_decompressor tells us whether it is read or write operation we can cast j_common_ptr to j_compress_ptr or j_decompress_ptr. Based on this I am creating jpeg_source_mgr or jpeg_destination_mgr object. Using this we can call RELEASE/GET_ARRAYS in sun_jpeg_output_message().
>
> Please find updated webrev for review:
> http://cr.openjdk.java.net/~jdv/8162461/webrev.01/
>
> Parallel to this I tried using two separate functions like  sun_jpeg_reader_output_message(j_decompress_ptr)  and sun_jpeg_writer_output_message(j_compress_ptr). But then we need to replicate error_exit() and emit_message() also to accept j_decompress_ptr and j_compress_ptr. It needs lot of changes at many places where we are using error_exit() and emit_message() in IJG library.
>
> Thanks,
> Jay
>
> -----Original Message-----
> From: Phil Race
> Sent: Wednesday, September 07, 2016 10:57 PM
> To: Jayathirth D V; 2d-dev
> Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-8162461: Hang due to JNI up-call made whilst holding JNI critical lock.
>
> This all looks reasonable. But I wonder if you missed something.
> Take a look at sun_jpeg_output_message(). That may also make up-calls.
> A pointer to this function is passed to the IJG library and I don't know the circumstances under which it may call this function.
> If it ever might do it whilst we are holding these locks then that would mean we'd need the same RELEASE/GET in there .. although the challenge would be that it does not have access to the data to release the arrays.
> So
> 1) Investigate if it is an actual issue,
> 2) If it is, then decide between a way to ensure the arrays are available to this function (looks tricky to me), or somehow deferring these up-calls or eliminating them.
>
> -phil.
>
> On 9/6/2016 11:49 PM, Jayathirth D V wrote:
>> Fixed typo.
>>
>> *From:* Jayathirth D V
>> *Sent:* Wednesday, September 07, 2016 12:11 PM
>> *To:* Philip Race; 2d-dev
>> *Subject:* [OpenJDK 2D-Dev] [9] RFR JDK-8162461: Hang due to JNI
>> up-call made whilst holding JNI critical lock.
>>
>> Hi,
>>
>> Please review the following fix in JDK9 at your convenience:
>>
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8162461
>>
>> Webrev : http://cr.openjdk.java.net/~jdv/8162461/webrev.00/
>> <http://cr.openjdk.java.net/%7Ejdv/8162461/webrev.00/>
>>
>> Issue : If we try to perform operations like
>> reader.abort()/reader.dispose()/ reader.reset() in any of the
>> IIOReadUpdateListener callbacks, JVM will throw deadlock error.
>>
>> Root cause : We are making callbacks from native side to Java by
>> holding some resources in JNI critical lock.
>>
>> Solution : We have to release the JNI critical lock on the resources
>> before we call Java function from native side. If we have JNI critical
>> lock and we throw an Exception in that case also we should release the
>> lock.
>>
>> Thanks,
>>
>> Jay
>>



More information about the 2d-dev mailing list