Permalink
Browse files

Merge pull request #2115 from jimfcarroll/fix-14005

Fix for 14005. GIL wasn't managed correctly in python spawned threads
  • Loading branch information...
2 parents 12af4b4 + 415fbde commit d0de086696743c17d25a05ef762ddc4000682794 @davilla davilla committed Jan 25, 2013
Showing with 46 additions and 25 deletions.
  1. +38 −23 xbmc/interfaces/python/PyContext.cpp
  2. +8 −2 xbmc/interfaces/python/PyContext.h
@@ -31,16 +31,18 @@ namespace XBMCAddon
{
struct PyContextState
{
- inline PyContextState() : value(0), state(NULL), gilReleasedDepth(0) {}
+ inline PyContextState(bool pcreatedByGilRelease = false) :
+ value(0), state(NULL), gilReleasedDepth(0), createdByGilRelease(pcreatedByGilRelease) {}
int value;
PyThreadState* state;
int gilReleasedDepth;
+ bool createdByGilRelease;
};
static XbmcThreads::ThreadLocal<PyContextState> tlsPyContextState;
- PyContext::PyContext()
+ void* PyContext::enterContext()
{
PyContextState* cur = tlsPyContextState.get();
if (cur == NULL)
@@ -51,9 +53,11 @@ namespace XBMCAddon
// increment the count
cur->value++;
+
+ return cur;
}
- PyContext::~PyContext()
+ void PyContext::leaveContext()
{
// here we ASSUME that the constructor was called.
PyContextState* cur = tlsPyContextState.get();
@@ -78,37 +82,48 @@ namespace XBMCAddon
void PyGILLock::releaseGil()
{
PyContextState* cur = tlsPyContextState.get();
- if (cur) // this means we're within the python context
+
+ // This means we're not within the python context, but
+ // because we may be in a thread spawned by python itself,
+ // we need to handle this.
+ if (!cur)
+ {
+ cur = (PyContextState*)PyContext::enterContext();
+ cur->createdByGilRelease = true;
+ }
+
+ if (cur->gilReleasedDepth == 0) // true if we are at the outermost
{
- if (cur->gilReleasedDepth == 0) // true if we are at the outermost
+ PyThreadState* _save;
+ // this macro sets _save
{
- PyThreadState* _save;
- // this macro sets _save
- {
- Py_UNBLOCK_THREADS
- }
- cur->state = _save;
+ Py_UNBLOCK_THREADS
}
- cur->gilReleasedDepth++; // the first time this goes to 1
+ cur->state = _save;
}
+ cur->gilReleasedDepth++; // the first time this goes to 1
}
void PyGILLock::acquireGil()
{
- PyContextState* cur = tlsPyContextState.get(); // this must be non-null given we released the Gil in the constructor
- if (cur) // we're within a PyContext
+ PyContextState* cur = tlsPyContextState.get();
+
+ // it's not possible for cur to be NULL (and if it is, we want to fail anyway).
+
+ // decrement the depth and make sure we're in the right place.
+ cur->gilReleasedDepth--;
+ if (cur->gilReleasedDepth == 0) // are we back to zero?
{
- // decrement the depth and make sure we're in the right place.
- cur->gilReleasedDepth--;
- if (cur->gilReleasedDepth == 0) // are we back to zero?
+ PyThreadState* _save = cur->state;
+ // This macros uses _save
{
- PyThreadState* _save = cur->state;
- // This macros uses _save
- {
- Py_BLOCK_THREADS
- }
- cur->state = NULL; // clear the state to indicate we've reacquired the gil
+ Py_BLOCK_THREADS
}
+ cur->state = NULL; // clear the state to indicate we've reacquired the gil
+
+ // we clear it only if we created it on this level.
+ if (cur->createdByGilRelease)
+ PyContext::leaveContext();
}
}
}
@@ -22,6 +22,8 @@ namespace XBMCAddon
{
namespace Python
{
+ class PyGILLock;
+
/**
* These classes should NOT be used with 'new'. They are expected to reside
* as stack instances and they act as "Guard" classes that track the
@@ -30,9 +32,13 @@ namespace XBMCAddon
class PyContext
{
protected:
+ friend class PyGILLock;
+ static void* enterContext();
+ static void leaveContext();
public:
- PyContext();
- ~PyContext();
+
+ inline PyContext() { enterContext(); }
+ inline ~PyContext() { leaveContext(); }
};
/**

0 comments on commit d0de086

Please sign in to comment.