Package: release.debian.org Severity: normal User: release.debian....@packages.debian.org Usertags: unblock
Hi, Please unblock package libsfml This fixes the important bug #855404 where SFML can in certain situations deadlock inside the GL context handling code. Upstream have explicitly asked me to try and include this fix into stretch. The patch originates from version 2.4.2 which is in experimental but was too late to get into stretch. I've tested the patch with reverse-dependencies in Debian and everything still works AFAIK. Thanks, James unblock libsfml/2.4.1+dfsg-3 -- System Information: Debian Release: 9.0 APT prefers unstable-debug APT policy: (500, 'unstable') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 4.9.0-1-amd64 (SMP w/4 CPU cores) Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Init: systemd (via /run/systemd/system)
diff -Nru libsfml-2.4.1+dfsg/debian/changelog
libsfml-2.4.1+dfsg/debian/changelog
--- libsfml-2.4.1+dfsg/debian/changelog 2016-12-30 19:02:05.000000000 +0000
+++ libsfml-2.4.1+dfsg/debian/changelog 2017-02-20 20:11:38.000000000 +0000
@@ -1,3 +1,9 @@
+libsfml (2.4.1+dfsg-3) unstable; urgency=medium
+
+ * Apply upstream patch to fix TransientContext deadlocks. (Closes: #855404)
+
+ -- James Cowgill <jcowg...@debian.org> Mon, 20 Feb 2017 20:11:38 +0000
+
libsfml (2.4.1+dfsg-2) unstable; urgency=medium
* Fix segfaults triggered by sf::Window::setIcon. (Closes: #849750)
diff -Nru
libsfml-2.4.1+dfsg/debian/patches/08_fix-transientcontext-deadlocks.patch
libsfml-2.4.1+dfsg/debian/patches/08_fix-transientcontext-deadlocks.patch
--- libsfml-2.4.1+dfsg/debian/patches/08_fix-transientcontext-deadlocks.patch
1970-01-01 01:00:00.000000000 +0100
+++ libsfml-2.4.1+dfsg/debian/patches/08_fix-transientcontext-deadlocks.patch
2017-02-20 20:11:38.000000000 +0000
@@ -0,0 +1,435 @@
+From 2857207cae8ccd8677ef3586add44102790dea92 Mon Sep 17 00:00:00 2001
+From: binary1248 <binary1...@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 -Nru libsfml-2.4.1+dfsg/debian/patches/series
libsfml-2.4.1+dfsg/debian/patches/series
--- libsfml-2.4.1+dfsg/debian/patches/series 2016-12-30 18:37:13.000000000
+0000
+++ libsfml-2.4.1+dfsg/debian/patches/series 2017-02-20 20:11:38.000000000
+0000
@@ -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
signature.asc
Description: OpenPGP digital signature
