On 27.01.2021 11:13, Johan Corveleyn wrote:
+void +OperationContext::closeTunnel(void *tunnel_context, void *) +{ + TunnelContext* tc = static_cast<TunnelContext*>(tunnel_context); + jobject jclosecb = tc->jclosecb; + delete tc; + + JNIEnv *env = JNIUtil::getEnv(); + if (JNIUtil::isJavaExceptionThrown()) + return; + + if (jclosecb) + callCloseTunnelCallback(env, jclosecb); }Here above in closeTunnel(), you dropped the early exit on if (!jclosecb) (before the call to JNIUtil::getEnv()). Was this intentional? Previously, in the case of a null jclosecb, the unnecessary calls to JNIUtil::getEnv() and JNIUtil::isJavaExceptionThrown() were also avoided. In the new version they aren't (not sure if that's important, but it's a small behavioral difference).
Yes, this is intentional. In r1882523, I further extend the cleanup by also cleaning 'jrequest' and 'jresponse'. In this case, early exit is no good, because everything needs to be cleaned independently. For the purpose of reducing the amount of diffs, I removed the early exit one patch earlier.
I believe that calling 'JNIUtil::getEnv()' and 'JNIUtil::isJavaExceptionThrown()' is merely a negligible performance change. I thought that it's not worthy to describe in commit message, but I could do that if you think otherwise.
