Skip to content

Commit

Permalink
Replaced TransientContextLock implementation with a more elaborate on…
Browse files Browse the repository at this point in the history 
…e which relies on locking a single mutex and thus avoids lock order inversion. Fixes SFML#1165.
  • Loading branch information
@binary1248
binary1248 committed Jan 27, 2017
1 parent 022f159 commit af5244d
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 123 deletions.
3 changes: 0 additions & 3 deletions include/SFML/Window/GlResource.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,6 @@ class SFML_WINDOW_API GlResource
///
////////////////////////////////////////////////////////////
~TransientContextLock();

private:
Context* m_context; ///< Temporary context, in case we needed to create one
};
};

Expand Down
206 changes: 145 additions & 61 deletions src/SFML/Window/GlContext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
// Headers
////////////////////////////////////////////////////////////
#include <SFML/Window/GlContext.hpp>
#include <SFML/Window/Context.hpp>
#include <SFML/System/ThreadLocalPtr.hpp>
#include <SFML/System/Mutex.hpp>
#include <SFML/System/Lock.hpp>
Expand Down Expand Up @@ -131,18 +132,70 @@ namespace
// We need to make sure that no operating system context
// or pixel format operations are performed simultaneously
// This mutex is also used to protect the shared context
// from being locked on multiple threads
// from being locked on multiple threads and for managing
// the resource count
sf::Mutex mutex;

// OpenGL resources counter
unsigned int resourceCount = 0;

// This per-thread variable holds the current context for each thread
sf::ThreadLocalPtr<sf::priv::GlContext> currentContext(NULL);

// The hidden, inactive context that will be shared with all other contexts
ContextType* sharedContext = NULL;

// This per-thread variable is set to point to the shared context
// if we had to acquire it when a TransientContextLock was required
sf::ThreadLocalPtr<sf::priv::GlContext> currentSharedContext(NULL);
// This structure contains all the state necessary to
// track TransientContext usage
struct TransientContext : private sf::NonCopyable
{
////////////////////////////////////////////////////////////
/// \brief Constructor
///
////////////////////////////////////////////////////////////
TransientContext() :
referenceCount (0),
context (0),
sharedContextLock(0),
useSharedContext (false)
{
if (resourceCount == 0)
{
context = new sf::Context;
}
else if (!currentContext)
{
sharedContextLock = new sf::Lock(mutex);
useSharedContext = true;
sharedContext->setActive(true);
}
}

////////////////////////////////////////////////////////////
/// \brief Destructor
///
////////////////////////////////////////////////////////////
~TransientContext()
{
if (useSharedContext)
sharedContext->setActive(false);

delete sharedContextLock;
delete context;
}

///////////////////////////////////////////////////////////
// Member data
////////////////////////////////////////////////////////////
unsigned int referenceCount;
sf::Context* context;
sf::Lock* sharedContextLock;
bool useSharedContext;
};

// This per-thread variable tracks if and how a transient
// context is currently being used on the current thread
sf::ThreadLocalPtr<TransientContext> transientContext(NULL);

// Supported OpenGL extensions
std::vector<std::string> extensions;
Expand All @@ -154,107 +207,138 @@ namespace sf
namespace priv
{
////////////////////////////////////////////////////////////
void GlContext::globalInit()
void GlContext::initResource()
{
// Protect from concurrent access
Lock lock(mutex);

if (sharedContext)
return;
// If this is the very first resource, trigger the global context initialization
if (resourceCount == 0)
{
if (sharedContext)
{
// Increment the resources counter
resourceCount++;

// Create the shared context
sharedContext = new ContextType(NULL);
sharedContext->initialize(ContextSettings());
return;
}

// Load our extensions vector
extensions.clear();
// Create the shared context
sharedContext = new ContextType(NULL);
sharedContext->initialize(ContextSettings());

// Check whether a >= 3.0 context is available
int majorVersion = 0;
glGetIntegerv(GL_MAJOR_VERSION, &majorVersion);
// Load our extensions vector
extensions.clear();

if (glGetError() == GL_INVALID_ENUM)
{
// Try to load the < 3.0 way
const char* extensionString = reinterpret_cast<const char*>(glGetString(GL_EXTENSIONS));
// Check whether a >= 3.0 context is available
int majorVersion = 0;
glGetIntegerv(GL_MAJOR_VERSION, &majorVersion);

do
if (glGetError() == GL_INVALID_ENUM)
{
const char* extension = extensionString;
// Try to load the < 3.0 way
const char* extensionString = reinterpret_cast<const char*>(glGetString(GL_EXTENSIONS));

while(*extensionString && (*extensionString != ' '))
extensionString++;
do
{
const char* extension = extensionString;

extensions.push_back(std::string(extension, extensionString));
}
while (*extensionString++);
}
else
{
// Try to load the >= 3.0 way
glGetStringiFuncType glGetStringiFunc = NULL;
glGetStringiFunc = reinterpret_cast<glGetStringiFuncType>(getFunction("glGetStringi"));
while(*extensionString && (*extensionString != ' '))
extensionString++;

if (glGetStringiFunc)
extensions.push_back(std::string(extension, extensionString));
}
while (*extensionString++);
}
else
{
int numExtensions = 0;
glGetIntegerv(GL_NUM_EXTENSIONS, &numExtensions);
// Try to load the >= 3.0 way
glGetStringiFuncType glGetStringiFunc = NULL;
glGetStringiFunc = reinterpret_cast<glGetStringiFuncType>(getFunction("glGetStringi"));

if (numExtensions)
if (glGetStringiFunc)
{
for (unsigned int i = 0; i < static_cast<unsigned int>(numExtensions); ++i)
int numExtensions = 0;
glGetIntegerv(GL_NUM_EXTENSIONS, &numExtensions);

if (numExtensions)
{
const char* extensionString = reinterpret_cast<const char*>(glGetStringiFunc(GL_EXTENSIONS, i));
for (unsigned int i = 0; i < static_cast<unsigned int>(numExtensions); ++i)
{
const char* extensionString = reinterpret_cast<const char*>(glGetStringiFunc(GL_EXTENSIONS, i));

extensions.push_back(extensionString);
extensions.push_back(extensionString);
}
}
}
}

// Deactivate the shared context so that others can activate it when necessary
sharedContext->setActive(false);
}

// Deactivate the shared context so that others can activate it when necessary
sharedContext->setActive(false);
// Increment the resources counter
resourceCount++;
}


////////////////////////////////////////////////////////////
void GlContext::globalCleanup()
void GlContext::cleanupResource()
{
// Protect from concurrent access
Lock lock(mutex);

if (!sharedContext)
return;
// Decrement the resources counter
resourceCount--;

// Destroy the shared context
delete sharedContext;
sharedContext = NULL;
// If there's no more resource alive, we can trigger the global context cleanup
if (resourceCount == 0)
{
if (!sharedContext)
return;

// Destroy the shared context
delete sharedContext;
sharedContext = NULL;
}
}


////////////////////////////////////////////////////////////
void GlContext::acquireTransientContext()
{
// If a capable context is already active on this thread
// there is no need to use the shared context for the operation
if (currentContext)
{
currentSharedContext = NULL;
return;
}
// Protect from concurrent access
Lock lock(mutex);

mutex.lock();
currentSharedContext = sharedContext;
sharedContext->setActive(true);
// If this is the first TransientContextLock on this thread
// construct the state object
if (!transientContext)
transientContext = new TransientContext;

// Increase the reference count
transientContext->referenceCount++;
}


////////////////////////////////////////////////////////////
void GlContext::releaseTransientContext()
{
if (!currentSharedContext)
return;
// Protect from concurrent access
Lock lock(mutex);

// Make sure a matching acquireTransientContext() was called
assert(transientContext);

sharedContext->setActive(false);
mutex.unlock();
// Decrease the reference count
transientContext->referenceCount--;

// If this is the last TransientContextLock that is released
// destroy the state object
if (transientContext->referenceCount == 0)
{
delete transientContext;
transientContext = NULL;
}
}


Expand Down
25 changes: 11 additions & 14 deletions src/SFML/Window/GlContext.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,28 +49,25 @@ class GlContext : NonCopyable
public:

////////////////////////////////////////////////////////////
/// \brief Perform the global initialization
/// \brief Perform resource initialization
///
/// This function is called once, before the very first OpenGL
/// resource is created. It makes sure that everything is ready
/// for contexts to work properly.
/// Note: this function doesn't need to be thread-safe, as it
/// can be called only once.
/// This function is called every time an OpenGL resource is
/// created. When the first resource is initialized, it makes
/// sure that everything is ready for contexts to work properly.
///
////////////////////////////////////////////////////////////
static void globalInit();
static void initResource();

////////////////////////////////////////////////////////////
/// \brief Perform the global cleanup
/// \brief Perform resource cleanup
///
/// This function is called after the very last OpenGL resource
/// is destroyed. It makes sure that everything that was
/// created by initialize() is properly released.
/// Note: this function doesn't need to be thread-safe, as it
/// can be called only once.
/// This function is called every time an OpenGL resource is
/// destroyed. When the last resource is destroyed, it makes
/// sure that everything that was created by initResource()
/// is properly released.
///
////////////////////////////////////////////////////////////
static void globalCleanup();
static void cleanupResource();

////////////////////////////////////////////////////////////
/// \brief Acquires a context for short-term use on the current thread
Expand Down
Loading

0 comments on commit af5244d

Please sign in to comment.