Skip to content

Commit

Permalink
tf: Ensure we drop the GIL if we hold it _before_ taking the singleto…
Browse files Browse the repository at this point in the history
…n creation

lock.  Otherwise if another thread takes the GIL during singleton creation and
then attempts to GetInstance(), we'll deadlock.

Fixes #3087

(Internal change: 2329963)
(Internal change: 2329985)
  • Loading branch information
gitamohr authored and pixar-oss committed Jun 5, 2024
1 parent dd65489 commit 31ddf45
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 0 deletions.
19 changes: 19 additions & 0 deletions pxr/base/tf/instantiateSingleton.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,25 @@
#include "pxr/base/tf/mallocTag.h"
#include "pxr/base/arch/demangle.h"

#include <memory>
#include <thread>

PXR_NAMESPACE_OPEN_SCOPE

// This GIL-releasing helper is implemented in singleton.cpp. We do it this way
// to avoid including the Python headers here.
struct Tf_SingletonPyGILDropper
{
TF_API
Tf_SingletonPyGILDropper();
TF_API
~Tf_SingletonPyGILDropper();
private:
#ifdef PXR_PYTHON_SUPPORT_ENABLED
std::unique_ptr<class TfPyLock> _pyLock;
#endif // PXR_PYTHON_SUPPORT_ENABLED
};

template <class T> std::atomic<T *> TfSingleton<T>::_instance;

template <class T>
Expand All @@ -51,6 +66,10 @@ TfSingleton<T>::_CreateInstance(std::atomic<T *> &instance)
TfAutoMallocTag2 tag("Tf", "TfSingleton::_CreateInstance",
"Create Singleton " + ArchGetDemangled<T>());

// Drop the GIL if we have it, before possibly locking to create the
// singleton instance.
Tf_SingletonPyGILDropper dropGIL;

// Try to take isInitializing false -> true. If we do it, then check to see
// if we don't yet have an instance. If we don't, then we get to create it.
// Otherwise we just wait until the instance shows up.
Expand Down
26 changes: 26 additions & 0 deletions pxr/base/tf/singleton.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,30 @@
// Licensed under the terms set forth in the LICENSE.txt file available at
// https://openusd.org/license.
//

#include "pxr/pxr.h"
#include "pxr/base/tf/singleton.h"
#include "pxr/base/tf/instantiateSingleton.h"
#include "pxr/base/tf/pyLock.h"

#include <memory>

PXR_NAMESPACE_OPEN_SCOPE

// This GIL-releasing helper is declared in tf/instantiateSingleton.h. It
// exists so that users of instantiateSingleton.h don't need to pull in the
// Python headers via tf/pyLock.h.

Tf_SingletonPyGILDropper::Tf_SingletonPyGILDropper()
{
#ifdef PXR_PYTHON_SUPPORT_ENABLED
if (PyGILState_Check()) {
_pyLock = std::make_unique<TfPyLock>();
_pyLock->BeginAllowThreads();
}
#endif // PXR_PYTHON_SUPPORT_ENABLED
}

Tf_SingletonPyGILDropper::~Tf_SingletonPyGILDropper() = default;

PXR_NAMESPACE_CLOSE_SCOPE

0 comments on commit 31ddf45

Please sign in to comment.