[libsfml] 02/03: Apply upstream patch to fix TransientContext deadlocks

James Cowgill jcowgill at moszumanska.debian.org
Mon Feb 20 20:24:58 UTC 2017


This is an automated email from the git hooks/post-receive script.

jcowgill pushed a commit to branch debian/unstable
in repository libsfml.

commit 0076529b27064f2a59acc147e0c92cc201c4824b
Author: James Cowgill <jcowgill at debian.org>
Date:   Thu Feb 16 09:25:44 2017 +0000

    Apply upstream patch to fix TransientContext deadlocks
    
    Closes: #855404
---
 .../08_fix-transientcontext-deadlocks.patch        | 435 +++++++++++++++++++++
 debian/patches/series                              |   1 +
 2 files changed, 436 insertions(+)

diff --git a/debian/patches/08_fix-transientcontext-deadlocks.patch b/debian/patches/08_fix-transientcontext-deadlocks.patch
new file mode 100644
index 0000000..9e15db4
--- /dev/null
+++ b/debian/patches/08_fix-transientcontext-deadlocks.patch
@@ -0,0 +1,435 @@
+From 2857207cae8ccd8677ef3586add44102790dea92 Mon Sep 17 00:00:00 2001
+From: binary1248 <binary1248 at hotmail.com>
+Date: Sun, 27 Nov 2016 18:31:21 +0100
+Subject: [PATCH] Replaced TransientContextLock implementation with a more
+ elaborate one which relies on locking a single mutex and thus avoids lock
+ order inversion. Fixes #1165.
+
+---
+ src/SFML/Window/GlContext.cpp  | 206 +++++++++++++++++++++++++++++------------
+ src/SFML/Window/GlContext.hpp  |  25 +++--
+ src/SFML/Window/GlResource.cpp |  48 +---------
+ 3 files changed, 161 insertions(+), 118 deletions(-)
+
+diff --git a/src/SFML/Window/GlContext.cpp b/src/SFML/Window/GlContext.cpp
+index 8ae4b3ab..d773ed00 100644
+--- a/src/SFML/Window/GlContext.cpp
++++ b/src/SFML/Window/GlContext.cpp
+@@ -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>
+@@ -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;
+@@ -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;
++    }
+ }
+ 
+ 
+diff --git a/src/SFML/Window/GlContext.hpp b/src/SFML/Window/GlContext.hpp
+index 55b6c1f1..757c2dc1 100644
+--- a/src/SFML/Window/GlContext.hpp
++++ b/src/SFML/Window/GlContext.hpp
+@@ -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
+diff --git a/src/SFML/Window/GlResource.cpp b/src/SFML/Window/GlResource.cpp
+index e9a9ecc9..e8169954 100644
+--- a/src/SFML/Window/GlResource.cpp
++++ b/src/SFML/Window/GlResource.cpp
+@@ -27,17 +27,6 @@
+ ////////////////////////////////////////////////////////////
+ #include <SFML/Window/GlResource.hpp>
+ #include <SFML/Window/GlContext.hpp>
+-#include <SFML/Window/Context.hpp>
+-#include <SFML/System/Mutex.hpp>
+-#include <SFML/System/Lock.hpp>
+-
+-
+-namespace
+-{
+-    // OpenGL resources counter and its mutex
+-    unsigned int count = 0;
+-    sf::Mutex mutex;
+-}
+ 
+ 
+ namespace sf
+@@ -45,37 +34,21 @@ namespace sf
+ ////////////////////////////////////////////////////////////
+ GlResource::GlResource()
+ {
+-    // Protect from concurrent access
+-    Lock lock(mutex);
+-
+-    // If this is the very first resource, trigger the global context initialization
+-    if (count == 0)
+-        priv::GlContext::globalInit();
+-
+-    // Increment the resources counter
+-    count++;
++    priv::GlContext::initResource();
+ }
+ 
+ 
+ ////////////////////////////////////////////////////////////
+ GlResource::~GlResource()
+ {
+-    // Protect from concurrent access
+-    Lock lock(mutex);
+-
+-    // Decrement the resources counter
+-    count--;
+-
+-    // If there's no more resource alive, we can trigger the global context cleanup
+-    if (count == 0)
+-        priv::GlContext::globalCleanup();
++    priv::GlContext::cleanupResource();
+ }
+ 
+ 
+ ////////////////////////////////////////////////////////////
+ void GlResource::ensureGlContext()
+ {
+-    // Empty function for ABI compatibility, use acquireTransientContext instead
++    // Empty function for ABI compatibility, use TransientContextLock instead
+ }
+ 
+ 
+@@ -83,13 +56,8 @@ void GlResource::ensureGlContext()
+ GlResource::TransientContextLock::TransientContextLock() :
+ m_context(0)
+ {
+-    Lock lock(mutex);
+-
+-    if (count == 0)
+-    {
+-        m_context = new Context;
+-        return;
+-    }
++    // m_context is no longer used
++    // Remove it when ABI can be broken
+ 
+     priv::GlContext::acquireTransientContext();
+ }
+@@ -98,12 +66,6 @@ m_context(0)
+ ////////////////////////////////////////////////////////////
+ GlResource::TransientContextLock::~TransientContextLock()
+ {
+-    if (m_context)
+-    {
+-        delete m_context;
+-        return;
+-    }
+-
+     priv::GlContext::releaseTransientContext();
+ }
+ 
+-- 
+2.11.0
+
diff --git a/debian/patches/series b/debian/patches/series
index 275896b..333f7a5 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -3,3 +3,4 @@
 05_build-doc-once.patch
 06_pkgconfig-freebsd.patch
 07_fix-crashing-in-sf-Window-setIcon.patch
+08_fix-transientcontext-deadlocks.patch

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-games/libsfml.git



More information about the Pkg-games-commits mailing list