Detach only external threads

[sfod]

Issue #, if available:

The issue this PR fixes appears on Apple platforms with dispatch queue event loop enabled and using GraalVM.

Currently, when a native callback is executed, crt-java assumes it always runs on a dispatch queue thread. So, at the beginning of a callback, the thread attaches to JVM, and at the end - detaches from JVM. However, sometimes callbacks can be actually executed in the main thread, like in the bootstrap create-destroy test. This leads to callbacks attempting to detach the main JVM thread from JVM.
When executing in JVM mode, it seems such requests are simply being ignored, otherwise we should have seen errors. But in GraalVM this leads to the following error:

[TEST START] software.amazon.awssdk.crt.test.ClientBootstrapTest#testCreateDestroy
Fatal error: Must either be at a safepoint or in native mode

and then crash.

Description of changes:

This PR does for dispatch queue threads what is basically already done for CRT threads: detachment will happen only if attachment actually happened at the beginning of a callback.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

bret will ask for changes

[bretambrose]

As annoying as it will be to do (given how often it's used), I'd prefer to see a slight change to how this is done:

Add a aws_jvm_thread_attachment_context struct (or similar name) that contains the env and the bool, ala:

struct aws_jvm_thread_attachment_context {
   JNIEnv *env;
   bool should_detach;
};

and have aws_jni_acquire_thread_env() return the struct by value. Then the pattern becomes:

   struct aws_jvm_thread_attachment_context attachment_context = aws_jni_acquire_thread_env(jvm);
   JNIEnv *env = attachment_context.env;

   ...

   aws_jni_release(thread_env(jvm, &attachment_context);

I prefer encapsulating the attachment state in one type rather than splitting into two (and maybe more in the future?) separate fields.

[bretambrose]

As annoying as it will be to do (given how often it's used), I'd prefer to see a slight change to how this is done:

Add a aws_jvm_thread_attachment_context struct (or similar name) that contains the env and the bool, ala:

struct aws_jvm_thread_attachment_context {
   JNIEnv *env;
   bool should_detach;
};

and have aws_jni_acquire_thread_env() return the struct by value. Then the pattern becomes:

   struct aws_jvm_thread_attachment_context attachment_context = aws_jni_acquire_thread_env(jvm);
   JNIEnv *env = attachment_context.env;

   ...

   aws_jni_release(thread_env(jvm, &attachment_context);

I prefer encapsulating the attachment state in one type rather than splitting into two (and maybe more in the future?) separate fields.

Or maybe struct aws_jvm_env or similar to make it more general along the lines of "this holds all the state we need to perform JNI calls from native.