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

[Philip Race]

+1

-phil.

On 9/29/16, 1:49 AM, Jayathirth D V wrote:

Hi Phil,

New changes are submitted in JDK9 under JDK-8166685.

Please find updated webrev for review in JDK8u:

http://cr.openjdk.java.net/~jdv/8162461.8u/webrev.01/ <http://cr.openjdk.java.net/%7Ejdv/8162461.8u/webrev.01/>

I have verified JCK, jtreg and JPRT for updated changes in JDK8u.

Thanks,

Jay

*From:*Philip Race
*Sent:* Friday, September 23, 2016 10:10 PM
*To:* Jayathirth D V
*Cc:* 2d-dev

*Subject:* Re: [OpenJDK 2D-Dev] [8u] RFR JDK-8162461: Hang due to JNI up-call made whilst holding JNI critical lock.

You can't re-use the 9 bug ID in 9. So you will need to create a new bug to fix

that in 9.

But for 8u it can be part of this fix .. it would be weird to knowingly check in

a problem just so you could fix it separately.


-phil.

On 9/23/16, 12:13 AM, Jayathirth D V wrote:

    Hi Phil,

    I assumed setjmp/longjmp functions to be internal error handling
    mechanism rather than C standard library functions which provide
    non-local jumps.

    I verified setjmp/longjmp usage in ImageIO JPEG context and we
    need that RELEASE_ARRAYS() call in writeImage() which I have removed.

    Does I have to create a new Bug and add the RELEASE_ARRAY() call
    in writeImage() or I have to check-in this change under
    JDK-8162461 only?

    After check-in of this modification I will raise new webrev for 8u
    backport.

    Thanks,

    Jay

    *From:*Philip Race
    *Sent:* Thursday, September 22, 2016 9:51 PM
    *To:* Jayathirth D V
    *Cc:* 2d-dev
    *Subject:* Re: [OpenJDK 2D-Dev] [8u] RFR JDK-8162461: Hang due to
    JNI up-call made whilst holding JNI critical lock.

    I see this is mostly what I approved for JDK9 but I noticed you
    made a change
    after I approved it and I did not see or approve the updated version.
    I am concerned about this comment you posted for the JDK 9 webrev :

    >Regarding  "2856         RELEASE_ARRAYS(env, data, (const JOCTET
    *)(dest->next_output_byte));".
    >We don't need this RELEASE_ARRAY() call at this place as we have
    not yet pinned any buffer.
    > So I have removed it.
    >Please find updated webrev for review :
    >http://cr.openjdk.java.net/~jdv/8162461/webrev.02/
    <http://cr.openjdk.java.net/%7Ejdv/8162461/webrev.02/>

    What makes you so sure we have not pinned a buffer by then ?
    setjmp is special. It may return from any point in the writing
    process.

    In other words that removal looks wrong to me.

    -phil.

    On 9/15/16, 1:29 AM, Jayathirth D V wrote:

        Hi,

        Please review the following backport to JDK-8u from JDK-9 at
        your convenience:

        JDK-9 review thread :
        http://mail.openjdk.java.net/pipermail/2d-dev/2016-September/007601.html


        Bug : https://bugs.openjdk.java.net/browse/JDK-8162461

        JDK-8u Webrev :
        http://cr.openjdk.java.net/~jdv/8162461.8u/webrev.00/
        <http://cr.openjdk.java.net/%7Ejdv/8162461.8u/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.

        I have verified jtreg, JCK and JPRT in JDK8u-dev also.

        Thanks,

        Jay