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

Bug#843324: marked as done (ghostscript crashes on some machines, much of the time)

Your message dated Mon, 18 Nov 2019 22:30:07 +0100
with message-id <157411260768.238883.8182046441171275056@auryn.jones.dk>
and subject line Re: Bug#843324: ghostscript crashes on some machines, much of the time
has caused the Debian Bug report #843324,
regarding ghostscript crashes on some machines, much of the time
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
843324: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=843324
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
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.

--- End Message ---
--- Begin Message --- 
Quoting Ian Jackson (2016-11-05 21:12:15)
> 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.

Closing as fixed. Thanks, Ian!

 - Jonas

-- 
 * Jonas Smedegaard - idealist & Internet-arkitekt
 * Tlf.: +45 40843136  Website: http://dr.jones.dk/

 [x] quote me freely  [ ] ask before reusing  [ ] keep private

Attachment: signature.asc
Description: signature


--- End Message ---
Reply to: