dcb3b7
From f793042f2bac2ace9a5c0030b47b41c4db561a5b Mon Sep 17 00:00:00 2001
dcb3b7
From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>
dcb3b7
Date: Fri, 6 Jun 2014 14:31:59 +0200
dcb3b7
Subject: [PATCH] Destroy {GDBM,NDBM,ODBM,SDBM}_File objects only from original
dcb3b7
 thread context
dcb3b7
MIME-Version: 1.0
dcb3b7
Content-Type: text/plain; charset=UTF-8
dcb3b7
Content-Transfer-Encoding: 8bit
dcb3b7
dcb3b7
This patch fixes a crash when destroing a hash tied to a *_File
dcb3b7
database after spawning a thread:
dcb3b7
dcb3b7
use Fcntl;
dcb3b7
use SDBM_File;
dcb3b7
use threads;
dcb3b7
tie(my %dbtest, 'SDBM_File', "test.db", O_RDWR|O_CREAT, 0666);
dcb3b7
threads->new(sub {})->join;
dcb3b7
dcb3b7
This crashed or paniced depending on how perl was configured.
dcb3b7
dcb3b7
Closes RT#61912.
dcb3b7
dcb3b7
Signed-off-by: Petr Písař <ppisar@redhat.com>
dcb3b7
---
dcb3b7
 ext/GDBM_File/GDBM_File.xs | 16 ++++++++++------
dcb3b7
 ext/NDBM_File/NDBM_File.xs | 16 ++++++++++------
dcb3b7
 ext/ODBM_File/ODBM_File.xs | 18 +++++++++++-------
dcb3b7
 ext/SDBM_File/SDBM_File.xs |  4 +++-
dcb3b7
 t/lib/dbmt_common.pl       | 35 +++++++++++++++++++++++++++++++++++
dcb3b7
 5 files changed, 69 insertions(+), 20 deletions(-)
dcb3b7
dcb3b7
diff --git a/ext/GDBM_File/GDBM_File.xs b/ext/GDBM_File/GDBM_File.xs
dcb3b7
index 33e08e2..7160f54 100644
dcb3b7
--- a/ext/GDBM_File/GDBM_File.xs
dcb3b7
+++ b/ext/GDBM_File/GDBM_File.xs
dcb3b7
@@ -13,6 +13,7 @@
dcb3b7
 #define store_value 3
dcb3b7
 
dcb3b7
 typedef struct {
dcb3b7
+	tTHX    owner;
dcb3b7
 	GDBM_FILE 	dbp ;
dcb3b7
 	SV *    filter[4];
dcb3b7
 	int     filtering ;
dcb3b7
@@ -89,6 +90,7 @@ gdbm_TIEHASH(dbtype, name, read_write, mode)
dcb3b7
 	    if ((dbp =  gdbm_open(name, GDBM_BLOCKSIZE, read_write, mode,
dcb3b7
 	       	     	          (FATALFUNC) croak_string))) {
dcb3b7
 	        RETVAL = (GDBM_File)safecalloc(1, sizeof(GDBM_File_type)) ;
dcb3b7
+		RETVAL->owner = aTHX;
dcb3b7
 		RETVAL->dbp = dbp ;
dcb3b7
 	    }
dcb3b7
 	    
dcb3b7
@@ -109,12 +111,14 @@ gdbm_DESTROY(db)
dcb3b7
 	PREINIT:
dcb3b7
 	int i = store_value;
dcb3b7
 	CODE:
dcb3b7
-	gdbm_close(db);
dcb3b7
-	do {
dcb3b7
-	    if (db->filter[i])
dcb3b7
-		SvREFCNT_dec(db->filter[i]);
dcb3b7
-	} while (i-- > 0);
dcb3b7
-	safefree(db);
dcb3b7
+	if (db && db->owner == aTHX) {
dcb3b7
+	    gdbm_close(db);
dcb3b7
+	    do {
dcb3b7
+		if (db->filter[i])
dcb3b7
+		    SvREFCNT_dec(db->filter[i]);
dcb3b7
+	    } while (i-- > 0);
dcb3b7
+	    safefree(db);
dcb3b7
+	}
dcb3b7
 
dcb3b7
 #define gdbm_FETCH(db,key)			gdbm_fetch(db->dbp,key)
dcb3b7
 datum_value
dcb3b7
diff --git a/ext/NDBM_File/NDBM_File.xs b/ext/NDBM_File/NDBM_File.xs
dcb3b7
index 52e60fc..af223e5 100644
dcb3b7
--- a/ext/NDBM_File/NDBM_File.xs
dcb3b7
+++ b/ext/NDBM_File/NDBM_File.xs
dcb3b7
@@ -33,6 +33,7 @@ END_EXTERN_C
dcb3b7
 #define store_value 3
dcb3b7
 
dcb3b7
 typedef struct {
dcb3b7
+	tTHX    owner;
dcb3b7
 	DBM * 	dbp ;
dcb3b7
 	SV *    filter[4];
dcb3b7
 	int     filtering ;
dcb3b7
@@ -71,6 +72,7 @@ ndbm_TIEHASH(dbtype, filename, flags, mode)
dcb3b7
 	    RETVAL = NULL ;
dcb3b7
 	    if ((dbp =  dbm_open(filename, flags, mode))) {
dcb3b7
 	        RETVAL = (NDBM_File)safecalloc(1, sizeof(NDBM_File_type));
dcb3b7
+		RETVAL->owner = aTHX;
dcb3b7
 		RETVAL->dbp = dbp ;
dcb3b7
 	    }
dcb3b7
 	    
dcb3b7
@@ -84,12 +86,14 @@ ndbm_DESTROY(db)
dcb3b7
 	PREINIT:
dcb3b7
 	int i = store_value;
dcb3b7
 	CODE:
dcb3b7
-	dbm_close(db->dbp);
dcb3b7
-	do {
dcb3b7
-	    if (db->filter[i])
dcb3b7
-		SvREFCNT_dec(db->filter[i]);
dcb3b7
-	} while (i-- > 0);
dcb3b7
-	safefree(db);
dcb3b7
+	if (db && db->owner == aTHX) {
dcb3b7
+	    dbm_close(db->dbp);
dcb3b7
+	    do {
dcb3b7
+		if (db->filter[i])
dcb3b7
+		    SvREFCNT_dec(db->filter[i]);
dcb3b7
+	    } while (i-- > 0);
dcb3b7
+	    safefree(db);
dcb3b7
+	}
dcb3b7
 
dcb3b7
 #define ndbm_FETCH(db,key)			dbm_fetch(db->dbp,key)
dcb3b7
 datum_value
dcb3b7
diff --git a/ext/ODBM_File/ODBM_File.xs b/ext/ODBM_File/ODBM_File.xs
dcb3b7
index d1ece7f..f7e00a0 100644
dcb3b7
--- a/ext/ODBM_File/ODBM_File.xs
dcb3b7
+++ b/ext/ODBM_File/ODBM_File.xs
dcb3b7
@@ -45,6 +45,7 @@ datum	nextkey(datum key);
dcb3b7
 #define store_value 3
dcb3b7
 
dcb3b7
 typedef struct {
dcb3b7
+	tTHX    owner;
dcb3b7
 	void * 	dbp ;
dcb3b7
 	SV *    filter[4];
dcb3b7
 	int     filtering ;
dcb3b7
@@ -112,6 +113,7 @@ odbm_TIEHASH(dbtype, filename, flags, mode)
dcb3b7
 	    }
dcb3b7
 	    dbp = (void*)(dbminit(filename) >= 0 ? &dbmrefcnt : 0);
dcb3b7
 	    RETVAL = (ODBM_File)safecalloc(1, sizeof(ODBM_File_type));
dcb3b7
+	    RETVAL->owner = aTHX;
dcb3b7
 	    RETVAL->dbp = dbp ;
dcb3b7
 	}
dcb3b7
 	OUTPUT:
dcb3b7
@@ -124,13 +126,15 @@ DESTROY(db)
dcb3b7
 	dMY_CXT;
dcb3b7
 	int i = store_value;
dcb3b7
 	CODE:
dcb3b7
-	dbmrefcnt--;
dcb3b7
-	dbmclose();
dcb3b7
-	do {
dcb3b7
-	    if (db->filter[i])
dcb3b7
-		SvREFCNT_dec(db->filter[i]);
dcb3b7
-	} while (i-- > 0);
dcb3b7
-	safefree(db);
dcb3b7
+	if (db && db->owner == aTHX) {
dcb3b7
+	    dbmrefcnt--;
dcb3b7
+	    dbmclose();
dcb3b7
+	    do {
dcb3b7
+		if (db->filter[i])
dcb3b7
+		    SvREFCNT_dec(db->filter[i]);
dcb3b7
+	    } while (i-- > 0);
dcb3b7
+	    safefree(db);
dcb3b7
+	}
dcb3b7
 
dcb3b7
 datum_value
dcb3b7
 odbm_FETCH(db, key)
dcb3b7
diff --git a/ext/SDBM_File/SDBM_File.xs b/ext/SDBM_File/SDBM_File.xs
dcb3b7
index 291e41b..0bdae9a 100644
dcb3b7
--- a/ext/SDBM_File/SDBM_File.xs
dcb3b7
+++ b/ext/SDBM_File/SDBM_File.xs
dcb3b7
@@ -10,6 +10,7 @@
dcb3b7
 #define store_value 3
dcb3b7
 
dcb3b7
 typedef struct {
dcb3b7
+	tTHX    owner;
dcb3b7
 	DBM * 	dbp ;
dcb3b7
 	SV *    filter[4];
dcb3b7
 	int     filtering ;
dcb3b7
@@ -49,6 +50,7 @@ sdbm_TIEHASH(dbtype, filename, flags, mode)
dcb3b7
 	    }
dcb3b7
 	    if (dbp) {
dcb3b7
 	        RETVAL = (SDBM_File)safecalloc(1, sizeof(SDBM_File_type));
dcb3b7
+		RETVAL->owner = aTHX;
dcb3b7
 		RETVAL->dbp = dbp ;
dcb3b7
 	    }
dcb3b7
 	    
dcb3b7
@@ -60,7 +62,7 @@ void
dcb3b7
 sdbm_DESTROY(db)
dcb3b7
 	SDBM_File	db
dcb3b7
 	CODE:
dcb3b7
-	if (db) {
dcb3b7
+	if (db && db->owner == aTHX) {
dcb3b7
 	    int i = store_value;
dcb3b7
 	    sdbm_close(db->dbp);
dcb3b7
 	    do {
dcb3b7
diff --git a/t/lib/dbmt_common.pl b/t/lib/dbmt_common.pl
dcb3b7
index 5d4098c..a0a4d52 100644
dcb3b7
--- a/t/lib/dbmt_common.pl
dcb3b7
+++ b/t/lib/dbmt_common.pl
dcb3b7
@@ -511,5 +511,40 @@ unlink <Op_dbmx*>, $Dfile;
dcb3b7
    unlink <Op1_dbmx*>;
dcb3b7
 }
dcb3b7
 
dcb3b7
+{
dcb3b7
+   # Check DBM back-ends do not destroy objects from then-spawned threads.
dcb3b7
+   # RT#61912.
dcb3b7
+   SKIP: {
dcb3b7
+      my $threads_count = 2;
dcb3b7
+      skip 'Threads are disabled', 3 + 2 * $threads_count
dcb3b7
+        unless $Config{usethreads};
dcb3b7
+      use_ok('threads');
dcb3b7
+
dcb3b7
+      my %h;
dcb3b7
+      unlink <Op1_dbmx*>;
dcb3b7
+
dcb3b7
+      my $db = tie %h, $DBM_Class, 'Op1_dbmx', $create, 0640;
dcb3b7
+      isa_ok($db, $DBM_Class);
dcb3b7
+
dcb3b7
+      for (1 .. 2) {
dcb3b7
+         ok(threads->create(
dcb3b7
+            sub {
dcb3b7
+               $SIG{'__WARN__'} = sub { fail(shift) }; # debugging perl panics
dcb3b7
+                        # report it by spurious TAP line
dcb3b7
+               1;
dcb3b7
+            }), "Thread $_ created");
dcb3b7
+      }
dcb3b7
+      for (threads->list) {
dcb3b7
+         is($_->join, 1, "A thread exited successfully");
dcb3b7
+      }
dcb3b7
+
dcb3b7
+      pass("Tied object survived exiting threads");
dcb3b7
+
dcb3b7
+      undef $db;
dcb3b7
+      untie %h;
dcb3b7
+      unlink <Op1_dbmx*>;
dcb3b7
+   }
dcb3b7
+}
dcb3b7
+
dcb3b7
 done_testing();
dcb3b7
 1;
dcb3b7
-- 
dcb3b7
1.9.3
dcb3b7