Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix for 14005. GIL wasn't managed correctly in python spawned threads #2115

Merged
merged 1 commit into from

3 participants

@jimfcarroll
Collaborator

In threads spawned within python the PyContext was never used. In those cases the releasing/acquiring of the Gil was simply skipped. Therefore scripts that looped over 'sleep' from a python created thread (a typical pattern in python services) would block all other python threads forever.

Closes #14005

Jim Carroll In threads spawned within python the PyContext was never used. In tho…
…se cases the releasing/acquiring of the Gil was simply skipped. Therefore scripts that looped over 'sleep' from a python created thread (a typical pattern in python services) would block all other python threads forever.
415fbde
@ronie
Collaborator

fixes all issues i came across so far.
didn't spot any regressions either.

@davilla davilla merged commit d0de086 into xbmc:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 24, 2013
  1. In threads spawned within python the PyContext was never used. In tho…

    Jim Carroll authored
    …se cases the releasing/acquiring of the Gil was simply skipped. Therefore scripts that looped over 'sleep' from a python created thread (a typical pattern in python services) would block all other python threads forever.
This page is out of date. Refresh to see the latest.
View
61 xbmc/interfaces/python/PyContext.cpp
@@ -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();
}
}
}
View
10 xbmc/interfaces/python/PyContext.h
@@ -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(); }
};
/**
Something went wrong with that request. Please try again.