On Wed, 23 Jun 2021 09:19:16 GMT, Maxim Kartashev 
<github.com+28651297+mkartas...@openjdk.org> wrote:

> Added an `ExceptionCheck()` followed by `ExceptionDescribe()` and 
> `ExceptionClear()` immediately after the Java calls made from the callback 
> function `ReadTTFontFileFunc()` in `freetypeScaler.c`. 
> 
> The exception(s) need to be cleared because we're not returning immediately 
> to Java that would've been able to handle them gracefully. And in order not 
> to loose the exception entirely (even though the return value would also 
> indicate an error condition), print out the exception with 
> `ExceptionDescribe()` to aid in debugging.

src/java.desktop/share/native/libfontmanager/freetypeScaler.c line 53:

> 51:         (*(env))->ExceptionDescribe(env);\
> 52:         (*(env))->ExceptionClear(env);   \
> 53:     }

https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html#exceptiondescribe

"The pending exception is cleared as a side-effect of calling this function"

So you certainly don't need both of these and I would prefer that Describe only 
be used if really debugging where we think there's a REAL chance of an 
exception rather than just to keep JNI happy.

And the upcall that is likely (readBlock) itself will log any IOException (and 
catch it) in the event of an I/O error so I really think this is unlikely to be 
useful here.

src/java.desktop/share/native/libfontmanager/freetypeScaler.c line 198:

> 196:                                           sunFontIDs.ttReadBlockMID,
> 197:                                           bBuffer, offset, numBytes);
> 198:             // This is a callback, we are not returning immediately to 
> Java and better report exceptions now

I think the comment is un-needed .. since the only reason to call 
CHECK_EXCEPTION() is because of this.
Same for the other case.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4572

Reply via email to