diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 487fa9fd31..5771f05025 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -5,6 +5,9 @@ A new header is inserted each time a *tag* is created. ## Next release +- gltfio: fixed null pointer exception seen with some Android clients. +- Engine now exposes its JobSystem to C++ clients. + ## v1.5.1 - Fixed "no texture bound" warning in WebGL. diff --git a/libs/gltfio/src/ResourceLoader.cpp b/libs/gltfio/src/ResourceLoader.cpp index 050855dbaa..9bd8216f94 100644 --- a/libs/gltfio/src/ResourceLoader.cpp +++ b/libs/gltfio/src/ResourceLoader.cpp @@ -563,7 +563,7 @@ void ResourceLoader::Impl::bindTextureToMaterial(const TextureSlot& tb) { bool ResourceLoader::Impl::createTextures(bool async) { // If any decoding jobs are still underway, wait for them to finish. - utils::JobSystem* js = utils::JobSystem::getJobSystem(); + utils::JobSystem* js = &mEngine->getJobSystem(); if (mDecoderRootJob) { js->waitAndRelease(mDecoderRootJob); mDecoderRootJob = nullptr; @@ -677,9 +677,8 @@ bool ResourceLoader::Impl::createTextures(bool async) { } ResourceLoader::Impl::~Impl() { - utils::JobSystem* js = utils::JobSystem::getJobSystem(); if (mDecoderRootJob) { - js->waitAndRelease(mDecoderRootJob); + mEngine->getJobSystem().waitAndRelease(mDecoderRootJob); } } diff --git a/libs/utils/include/utils/JobSystem.h b/libs/utils/include/utils/JobSystem.h index 079bdf16fb..f62e2ff887 100644 --- a/libs/utils/include/utils/JobSystem.h +++ b/libs/utils/include/utils/JobSystem.h @@ -24,6 +24,8 @@ #include #include +#include + #include #include #include @@ -32,7 +34,6 @@ #include #include #include -#include #include namespace utils { @@ -88,10 +89,6 @@ public: void emancipate(); - // return the JobSystem this thread is associated with. nullptr if this thread is not - // part of a Jobsystem. - static JobSystem* getJobSystem() noexcept; - // If a parent is not specified when creating a job, that job will automatically take the // master job as a parent. // The master job is reset when waited on. @@ -341,7 +338,7 @@ private: static_assert(sizeof(ThreadState) % CACHELINE_SIZE == 0, "ThreadState doesn't align to a cache line"); - static ThreadState& getState() noexcept; + ThreadState& getState() noexcept; void incRef(Job const* job) noexcept; void decRef(Job const* job) noexcept; @@ -406,7 +403,7 @@ private: uint8_t mParallelSplitCount = 0; // # of split allowable in parallel_for Job* mMasterJob = nullptr; - static UTILS_DECLARE_TLS(ThreadState *) sThreadState; + tsl::robin_map mThreadMap; }; // ------------------------------------------------------------------------------------------------- diff --git a/libs/utils/src/JobSystem.cpp b/libs/utils/src/JobSystem.cpp index cf467362c4..5d8b4286a3 100644 --- a/libs/utils/src/JobSystem.cpp +++ b/libs/utils/src/JobSystem.cpp @@ -66,15 +66,13 @@ namespace utils { -UTILS_DEFINE_TLS(JobSystem::ThreadState *) JobSystem::sThreadState(nullptr); - void JobSystem::setThreadName(const char* name) noexcept { #if defined(__linux__) pthread_setname_np(pthread_self(), name); #elif defined(__APPLE__) pthread_setname_np(name); #else -// TODO: implement setting thread name on WIN32 +// TODO: implement setting thread name on WIN32 #endif } @@ -190,11 +188,6 @@ void JobSystem::decRef(Job const* job) noexcept { } } -JobSystem* JobSystem::getJobSystem() noexcept { - ThreadState* const state = sThreadState; - return state ? state->js : nullptr; -} - void JobSystem::requestExit() noexcept { mExitRequested.store(true); { std::lock_guard lock(mWaiterLock); } @@ -230,9 +223,9 @@ void JobSystem::wake() noexcept { } inline JobSystem::ThreadState& JobSystem::getState() noexcept { - // check we're not using a thread not owned by the thread pool - assert(sThreadState); - return *sThreadState; + auto iter = mThreadMap.find(std::this_thread::get_id()); + ASSERT_PRECONDITION(iter != mThreadMap.end(), "This thread has not been adopted."); + return *iter->second; } JobSystem::Job* JobSystem::allocateJob() noexcept { @@ -307,8 +300,9 @@ void JobSystem::loop(ThreadState* state) noexcept { // to core. On Android, it looks like the affinity needs to be reset from time to time. setThreadAffinityById(state->id); - // record our work queue to thread-local storage - sThreadState = state; + // record our work queue + bool inserted = mThreadMap.emplace(std::this_thread::get_id(), state).second; + ASSERT_PRECONDITION(inserted, "This thread is already in a loop."); // run our main loop... do { @@ -474,7 +468,9 @@ void JobSystem::runAndWait(JobSystem::Job*& job) noexcept { } void JobSystem::adopt() { - ThreadState* const state = sThreadState; + const auto tid = std::this_thread::get_id(); + auto iter = mThreadMap.find(tid); + ThreadState* const state = iter == mThreadMap.end() ? nullptr : iter->second; if (state) { // we're already part of a JobSystem, do nothing. ASSERT_PRECONDITION(this == state->js, @@ -497,14 +493,16 @@ void JobSystem::adopt() { // however, it's not a problem since mThreadState is pre-initialized and valid // (e.g.: the queue is empty). - sThreadState = &mThreadStates[index]; + mThreadMap[tid] = &mThreadStates[index]; } void JobSystem::emancipate() { - ThreadState* const state = sThreadState; + const auto tid = std::this_thread::get_id(); + auto iter = mThreadMap.find(tid); + ThreadState* const state = iter == mThreadMap.end() ? nullptr : iter->second; ASSERT_PRECONDITION(state, "this thread is not an adopted thread"); ASSERT_PRECONDITION(state->js == this, "this thread is not adopted by us"); - sThreadState = nullptr; + mThreadMap.erase(iter); } io::ostream& operator<<(io::ostream& out, JobSystem const& js) {