juce_android_JNI bugfix?

Android
1

 

juce_android_JNIHelpers.h. Claim you need locks around get functions too.

 

class ThreadLocalJNIEnvHolder
{
public:
    ThreadLocalJNIEnvHolder()
        : jvm (nullptr)
    {
        zeromem (threads, sizeof (threads));
        zeromem (envs, sizeof (envs));
    }
    void initialise (JNIEnv* env)
    {
        // NB: the DLL can be left loaded by the JVM, so the same static
        // objects can end up being reused by subsequent runs of the app
        zeromem (threads, sizeof (threads));
        zeromem (envs, sizeof (envs));
        env->GetJavaVM (&jvm);
        _add(env);
    }
    JNIEnv* attach()
    {
        JNIEnv* env = _attach();

        if (env != nullptr)
        {
            SpinLock::ScopedLockType sl (addRemoveLock);
            if (!_get())
                _add(env);
        }
        return env;
    }
    void detach()
    {
        jvm->DetachCurrentThread();
        const pthread_t thisThread = pthread_self();
        SpinLock::ScopedLockType sl (addRemoveLock);
        for (int i = 0; i < maxThreads; ++i)
        {
            if (threads[i] == thisThread)
            {
                threads[i] = 0;
                break;
            }
        }
    }
    JNIEnv* getOrAttach() noexcept
    {
        SpinLock::ScopedLockType sl (addRemoveLock);
        JNIEnv* env = _get();
        if (env == nullptr)
        {
            env = _attach();
            if (env) _add(env);
        }
        jassert (env != nullptr);
        return env;
    }
    enum { maxThreads = 32 };
private:
    JavaVM* jvm;
    pthread_t threads [maxThreads];
    JNIEnv* envs [maxThreads];
    SpinLock addRemoveLock;
    JNIEnv* _attach()
    {
        JNIEnv* env = nullptr;
        jvm->AttachCurrentThread (&env, nullptr);
        return env;
    }
    JNIEnv* _get() const noexcept
    {
        const pthread_t thisThread = pthread_self();
        for (int i = 0; i < maxThreads; ++i)
            if (threads[i] == thisThread)
                return envs[i];
        return nullptr;
    }
    void _add(JNIEnv* env)
    {

        const pthread_t thisThread = pthread_self();

        for (int i = 0; i < maxThreads; ++i)
        {
            if (threads[i] == 0)
            {
                envs[i] = env;
                threads[i] = thisThread;
                return;
            }
        }
        jassertfalse; // too many threads!
    }
};
2

Hmm. Yes, it could use a bit of locking in getOrAttach, but it's also performance-critical so would probably benefit from an unlocked fast-path. Try what I've checked in now..

3

so now im not sure we *do* need the lock in get after all. Presumably, in `getOrAttach' if its not in the list, it can't put put into the list by anyone else.

 

Turns out that my app was crashing because `attach' was attaching a thread that was *already* in the list. This was because the same thread has called `getOrAttach' earlier. I'm still not sure how this can happen. So i made attach() return `getOrAttach()' and it works better, but not perfectly.

What happens if a java thread calls into Juce and *then* calls back into java. does this thread get "attached" although it's not a juce thread?

 

4

But attach() is only ever be called when a thread starts-up..(?) It's done in the pthread launch code. I can't see how it could be called for a thread that already exists?

5

 

nor do i, but that's what appeared to be happening. how are the thread ids allocated, perhaps a java origin thread has a bogus ID.