[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#843324: ghostscript crashes on some machines, much of the time



Package: ghostscript
Version: 9.06~dfsg-2+deb8u4
Tags: patch
Severity: serious
Control: fixed -1 9.19~dfsg-3.1

On some machines, with recent jessie libcs, on amd64 Linux (at least),
ghostscript crashes because it unlocks an already-unlocked pthread
DEFAULT mutex.  (Well, actually, in my test case, it locks it twice,
and then unlocks it twice.)

This is the upstream bug:
  http://bugs.ghostscript.com/show_bug.cgi?id=695862

It can be fixed by cherry-picking the corresponding upstream patch,
  444e0bf9c43b "Bug 695862: use PTHREAD_MUTEX_RECURSIVE(_NP) if available"

Please find attached a suitable backport.  I threw away the changes to
the configure and build system.  We do not need these because we
always have recursive mutexes available: so instead, I just definen
the appropriate preprocessor symbol in the file which uses it.

Thanks.

Ian.

>From f96eb410d8c1b6f7660638c39c9add82be158d94 Mon Sep 17 00:00:00 2001
From: Chris Liddell <chris.liddell@artifex.com>
Date: Mon, 16 Mar 2015 12:52:49 +0000
Subject: [PATCH] Bug 695862: use PTHREAD_MUTEX_RECURSIVE(_NP) if available

or properly emulate recursive mutexes ourselves.

No cluster differences

(cherry picked from commit 444e0bf9c43bae0261660e6318ba0e514c18d41e)

Conflicts:
	config.mak.in
	configure.ac
	gs/Makefile.in
	gs/configure.ac

[ Dropped all the buildsystem and configure changes.  Instead,
  we just hardcode GS_RECURSIVE_MUTEXATTR since it will
  always be available on Debian. -iwj ]
---
 base/gp_psync.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 59 insertions(+), 12 deletions(-)

diff --git a/base/gp_psync.c b/base/gp_psync.c
index 60f6977..d09871b 100644
--- a/base/gp_psync.c
+++ b/base/gp_psync.c
@@ -13,6 +13,7 @@
    CA  94903, U.S.A., +1(415)492-9861, for further information.
 */
 
+#define GS_RECURSIVE_MUTEXATTR 1 /* always, on Debian */
 
 /* POSIX pthreads threads / semaphore / monitor implementation */
 #include "std.h"
@@ -128,13 +129,20 @@ gp_semaphore_signal(gp_semaphore * sema)
 /* Monitor supports enter/leave semantics */
 
 /*
- * We need PTHREAD_MUTEX_RECURSIVE behavior, but this isn't totally portable
- * so we implement it in a more portable fashion, keeping track of the
- * owner thread using 'pthread_self()'
+ * We need PTHREAD_MUTEX_RECURSIVE behavior, but this isn't
+ * supported on all pthread platforms, so if it's available
+ * we'll use it, otherwise we'll emulate it.
+ * GS_RECURSIVE_MUTEXATTR is set by the configure script
+ * on Unix-like machines to the attribute setting for
+ * PTHREAD_MUTEX_RECURSIVE - on linux this is usually
+ * PTHREAD_MUTEX_RECURSIVE_NP
  */
 typedef struct gp_pthread_recursive_s {
     pthread_mutex_t mutex;	/* actual mutex */
+#ifndef GS_RECURSIVE_MUTEXATTR
     pthread_t	self_id;	/* owner */
+    int lcount;
+#endif
 } gp_pthread_recursive_t;
 
 uint
@@ -148,12 +156,32 @@ gp_monitor_open(gp_monitor * mona)
 {
     pthread_mutex_t *mon;
     int scode;
+    pthread_mutexattr_t attr;
+    pthread_mutexattr_t *attrp = NULL;
 
     if (!mona)
         return -1;		/* monitors are not movable */
-    mon = &((gp_pthread_recursive_t *)mona)->mutex;
+
+
+#ifdef GS_RECURSIVE_MUTEXATTR
+    attrp = &attr;
+    scode = pthread_mutexattr_init(attrp);
+    if (scode < 0) goto done;
+
+    scode = pthread_mutexattr_settype(attrp, GS_RECURSIVE_MUTEXATTR);
+    if (scode < 0) {
+        goto done;
+    }
+#else     
     ((gp_pthread_recursive_t *)mona)->self_id = 0;	/* Not valid unless mutex is locked */
-    scode = pthread_mutex_init(mon, NULL);
+    ((gp_pthread_recursive_t *)mona)->lcount = 0;
+#endif
+
+    mon = &((gp_pthread_recursive_t *)mona)->mutex;
+    scode = pthread_mutex_init(mon, attrp);
+    if (attrp)
+        (void)pthread_mutexattr_destroy(attrp);
+done:
     return SEM_ERROR_CODE(scode);
 }
 
@@ -173,29 +201,48 @@ gp_monitor_enter(gp_monitor * mona)
     pthread_mutex_t * const mon = (pthread_mutex_t *)mona;
     int scode;
 
+#ifdef GS_RECURSIVE_MUTEXATTR
+    scode = pthread_mutex_lock(mon);
+#else
     if ((scode = pthread_mutex_trylock(mon)) == 0) {
         ((gp_pthread_recursive_t *)mona)->self_id = pthread_self();
-        return SEM_ERROR_CODE(scode);
+        ((gp_pthread_recursive_t *)mona)->lcount++;
     } else {
-        if (pthread_equal(pthread_self(),((gp_pthread_recursive_t *)mona)->self_id))
-            return 0;
+        if (pthread_equal(pthread_self(),((gp_pthread_recursive_t *)mona)->self_id)) {
+            ((gp_pthread_recursive_t *)mona)->lcount++;
+            scode = 0;
+        }
         else {
             /* we were not the owner, wait */
             scode = pthread_mutex_lock(mon);
             ((gp_pthread_recursive_t *)mona)->self_id = pthread_self();
-            return SEM_ERROR_CODE(scode);
+            ((gp_pthread_recursive_t *)mona)->lcount++;
         }
     }
+#endif
+    return SEM_ERROR_CODE(scode);
 }
 
 int
 gp_monitor_leave(gp_monitor * mona)
 {
     pthread_mutex_t * const mon = (pthread_mutex_t *)mona;
-    int scode;
+    int scode = 0;
 
-    scode = pthread_mutex_unlock(mon);
-    ((gp_pthread_recursive_t *)mona)->self_id = 0;	/* Not valid unless mutex is locked */
+#ifdef GS_RECURSIVE_MUTEXATTR
+    scode = pthread_mutex_lock(mon);
+#else
+    if (pthread_equal(pthread_self(),((gp_pthread_recursive_t *)mona)->self_id)) {
+      if ((--((gp_pthread_recursive_t *)mona)->lcount) == 0) {
+          scode = pthread_mutex_unlock(mon);
+          ((gp_pthread_recursive_t *)mona)->self_id = 0;	/* Not valid unless mutex is locked */
+
+      }
+    }
+    else {
+        scode = -1 /* should be EPERM */;
+    }
+#endif
     return SEM_ERROR_CODE(scode);
 }
 
-- 
2.1.4

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

Reply to: