JavaKit: thread detach interactions with implicitly created threads

[lhoward]

Considering the not uncommon case where an application uses structured concurrency and some or all of the executors use libdispatch. The application does not manage thread creation and destruction explicitly. (Naturally are other reasons to use libdispatch, thread pools, etc, but this case is noteworthy as it is generally imported by the language runtime itself.)

JNI's AttachCurrentThread() is called implicitly by JavaKit when retrieving the environment (see also #157) but DetachCurrentThread() must currently be called explicitly via its JavaVirtualMachine wrapper. Failing to detach the thread can cause bad things to happen (leaking local references, thread monitors hanging, etc - although admittedly neither of these should happen on a dispatch queue using JavaKit).

I suspect, on attach, we should register a dummy TSD with a cleanup handler that will detach the current thread from the JVM, on thread exit or cancellation. Whilst this would require the introduction of some non-portable code I think it speaks to leaving things the ’way we found them’.

(Interestingly side note – I see libdispatch has _dispatch_install_thread_detach_callback() for Android only, anyone know the provenance of this?)

[ktoso]

(Interestingly side note – I see libdispatch has _dispatch_install_thread_detach_callback() for Android only, anyone know the provenance of this?)

Okey this may save the day for Android specifically... I don't know much about it though but if it's there in corelibs dispatch we can use it I think. I'll ask around a bit.

[lhoward]

(Interestingly side note – I see libdispatch has _dispatch_install_thread_detach_callback() for Android only, anyone know the provenance of this?)

Okey this may save the day for Android specifically... I don't know much about it though but if it's there in corelibs dispatch we can use it I think. I'll ask around a bit.

I wonder it may have been used for some thing Apple-private as I can't find any references in the open source Foundation.

In any case, if we have the opportunity to call _dispatch_install_thread_detach_callback(), we also have the opportunity to call pthread_setspecific() (or equivalent), and presumably we should favour the more portable API.

[ktoso]

Hmm, yeah I think we can do that with just the callbacks to pthread_setspecific, but I'll check in if there's better reasons for the detach callback or not.

[lhoward]

Yeah, _dispatch_install_thread_detach_callback() could be called at program startup I suppose, it's more like __cxa_thread_atexit(). But I don't think that particularly helps us given JavaKit attaches on demand.

(I neglected to note it's exposed as setThreadDetachCallback() in Swift, so that's another symbol to look for.)

[lhoward]

FWIW, Android (or at least, some bits of it – I can't promise this is the only path) appears to do what was proposed, i.e. attach on demand and detach in TLS.

static inline JNIEnv* GetOrAttachJNIEnvironment(JavaVM* jvm, jint version = JNI_VERSION_1_4) {
    JNIEnv* env = GetJNIEnvironment(jvm, version);
    if (!env) {
        int result = jvm->AttachCurrentThread(&env, nullptr);
        LOG_ALWAYS_FATAL_IF(result != JNI_OK, "JVM thread attach failed.");
        struct VmDetacher {
            VmDetacher(JavaVM* vm) : mVm(vm) {}
            ~VmDetacher() { mVm->DetachCurrentThread(); }

        private:
            JavaVM* const mVm;
        };
        static thread_local VmDetacher detacher(jvm);
    }
    return env;
}