f47bb1
From bfb2cb3cddffa144b521bb5dff76af1e065288ad Mon Sep 17 00:00:00 2001
f47bb1
From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>
f47bb1
Date: Tue, 10 Jun 2014 14:28:09 +0200
f47bb1
Subject: [PATCH] Destroy DB_File objects only from original thread context
f47bb1
MIME-Version: 1.0
f47bb1
Content-Type: text/plain; charset=UTF-8
f47bb1
Content-Transfer-Encoding: 8bit
f47bb1
f47bb1
This patch fixes a crash when destroing a hash tied to a DB_File
f47bb1
database after spawning a thread:
f47bb1
f47bb1
use Fcntl;
f47bb1
use DB_File;
f47bb1
use threads;
f47bb1
tie(my %dbtest, 'DB_File', "test.db", O_RDWR|O_CREAT, 0666);
f47bb1
threads->new(sub {})->join;
f47bb1
f47bb1
This crashed or paniced depending on how perl was configured.
f47bb1
f47bb1
Closes RT#61912.
f47bb1
f47bb1
Signed-off-by: Petr Písař <ppisar@redhat.com>
f47bb1
---
f47bb1
 DB_File.xs     | 50 +++++++++++++++++++++++++++++++-------------------
f47bb1
 MANIFEST       |  1 +
f47bb1
 t/db-threads.t | 46 ++++++++++++++++++++++++++++++++++++++++++++++
f47bb1
 3 files changed, 78 insertions(+), 19 deletions(-)
f47bb1
 create mode 100644 t/db-threads.t
f47bb1
f47bb1
diff --git a/DB_File.xs b/DB_File.xs
f47bb1
index f417b22..ed6a904 100755
f47bb1
--- a/DB_File.xs
f47bb1
+++ b/DB_File.xs
f47bb1
@@ -407,6 +407,7 @@ typedef union INFO {
f47bb1
 
f47bb1
 typedef struct {
f47bb1
 	DBTYPE	type ;
f47bb1
+	tTHX    owner ;
f47bb1
 	DB * 	dbp ;
f47bb1
 	SV *	compare ;
f47bb1
 	bool	in_compare ;
f47bb1
@@ -1006,6 +1007,7 @@ SV *   sv ;
f47bb1
 		    name, flags, mode, sv == NULL) ;  
f47bb1
 #endif
f47bb1
     Zero(RETVAL, 1, DB_File_type) ;
f47bb1
+    RETVAL->owner = aTHX;
f47bb1
 
f47bb1
     /* Default to HASH */
f47bb1
     RETVAL->filtering = 0 ;
f47bb1
@@ -1278,6 +1280,7 @@ SV *   sv ;
f47bb1
 
f47bb1
 /* printf("In ParseOpenInfo name=[%s] flags=[%d] mode = [%d]\n", name, flags, mode) ;  */
f47bb1
     Zero(RETVAL, 1, DB_File_type) ;
f47bb1
+    RETVAL->owner = aTHX;
f47bb1
 
f47bb1
     /* Default to HASH */
f47bb1
     RETVAL->filtering = 0 ;
f47bb1
@@ -1597,27 +1600,36 @@ db_DESTROY(db)
f47bb1
 	INIT:
f47bb1
 	  CurrentDB = db ;
f47bb1
 	  Trace(("DESTROY %p\n", db));
f47bb1
-	CLEANUP:
f47bb1
-	  Trace(("DESTROY %p done\n", db));
f47bb1
-	  if (db->hash)
f47bb1
-	    SvREFCNT_dec(db->hash) ;
f47bb1
-	  if (db->compare)
f47bb1
-	    SvREFCNT_dec(db->compare) ;
f47bb1
-	  if (db->prefix)
f47bb1
-	    SvREFCNT_dec(db->prefix) ;
f47bb1
-	  if (db->filter_fetch_key)
f47bb1
-	    SvREFCNT_dec(db->filter_fetch_key) ;
f47bb1
-	  if (db->filter_store_key)
f47bb1
-	    SvREFCNT_dec(db->filter_store_key) ;
f47bb1
-	  if (db->filter_fetch_value)
f47bb1
-	    SvREFCNT_dec(db->filter_fetch_value) ;
f47bb1
-	  if (db->filter_store_value)
f47bb1
-	    SvREFCNT_dec(db->filter_store_value) ;
f47bb1
-	  safefree(db) ;
f47bb1
+	CODE:
f47bb1
+	  RETVAL = 0;
f47bb1
+	  if (db && db->owner == aTHX) {
f47bb1
+	    RETVAL = db_DESTROY(db);
f47bb1
 #ifdef DB_VERSION_MAJOR
f47bb1
-	  if (RETVAL > 0)
f47bb1
-	    RETVAL = -1 ;
f47bb1
+	    if (RETVAL > 0)
f47bb1
+		RETVAL = -1 ;
f47bb1
 #endif
f47bb1
+	  }
f47bb1
+	OUTPUT:
f47bb1
+	  RETVAL
f47bb1
+	CLEANUP:
f47bb1
+	  Trace(("DESTROY %p done\n", db));
f47bb1
+	  if (db && db->owner == aTHX) {
f47bb1
+	    if (db->hash)
f47bb1
+		SvREFCNT_dec(db->hash) ;
f47bb1
+	    if (db->compare)
f47bb1
+		SvREFCNT_dec(db->compare) ;
f47bb1
+	    if (db->prefix)
f47bb1
+		SvREFCNT_dec(db->prefix) ;
f47bb1
+	    if (db->filter_fetch_key)
f47bb1
+		SvREFCNT_dec(db->filter_fetch_key) ;
f47bb1
+	    if (db->filter_store_key)
f47bb1
+		SvREFCNT_dec(db->filter_store_key) ;
f47bb1
+	    if (db->filter_fetch_value)
f47bb1
+		SvREFCNT_dec(db->filter_fetch_value) ;
f47bb1
+	    if (db->filter_store_value)
f47bb1
+		SvREFCNT_dec(db->filter_store_value) ;
f47bb1
+	    safefree(db) ;
f47bb1
+	  }
f47bb1
 
f47bb1
 
f47bb1
 int
f47bb1
diff --git a/MANIFEST b/MANIFEST
f47bb1
index e460e81..47f43f7 100644
f47bb1
--- a/MANIFEST
f47bb1
+++ b/MANIFEST
f47bb1
@@ -27,6 +27,7 @@ t/db-btree.t
f47bb1
 t/db-hash.t
f47bb1
 t/db-recno.t
f47bb1
 t/pod.t
f47bb1
+t/db-threads.t
f47bb1
 typemap
f47bb1
 version.c
f47bb1
 META.yml                                 Module meta-data (added by MakeMaker)
f47bb1
diff --git a/t/db-threads.t b/t/db-threads.t
f47bb1
new file mode 100644
f47bb1
index 0000000..b9f69b6
f47bb1
--- /dev/null
f47bb1
+++ b/t/db-threads.t
f47bb1
@@ -0,0 +1,46 @@
f47bb1
+#!./perl
f47bb1
+
f47bb1
+use warnings;
f47bb1
+use strict;
f47bb1
+use Config;
f47bb1
+use Fcntl;
f47bb1
+use Test::More;
f47bb1
+use DB_File;
f47bb1
+
f47bb1
+if (-d "lib" && -f "TEST") {
f47bb1
+    if ($Config{'extensions'} !~ /\bDB_File\b/ ) {
f47bb1
+        plan skip_all => 'DB_File was not built';
f47bb1
+    }
f47bb1
+}
f47bb1
+plan skip_all => 'Threads are disabled'
f47bb1
+    unless $Config{usethreads};
f47bb1
+
f47bb1
+plan tests => 7;
f47bb1
+
f47bb1
+# Check DBM back-ends do not destroy objects from then-spawned threads.
f47bb1
+# RT#61912.
f47bb1
+use_ok('threads');
f47bb1
+
f47bb1
+my %h;
f47bb1
+unlink <threads*>;
f47bb1
+
f47bb1
+my $db = tie %h, 'DB_File', 'threads', O_RDWR|O_CREAT, 0640;
f47bb1
+isa_ok($db, 'DB_File');
f47bb1
+
f47bb1
+for (1 .. 2) {
f47bb1
+    ok(threads->create(
f47bb1
+        sub {
f47bb1
+            $SIG{'__WARN__'} = sub { fail(shift) }; # debugging perl panics
f47bb1
+                # report it by spurious TAP line
f47bb1
+            1;
f47bb1
+        }), "Thread $_ created");
f47bb1
+}
f47bb1
+for (threads->list) {
f47bb1
+    is($_->join, 1, "A thread exited successfully");
f47bb1
+}
f47bb1
+
f47bb1
+pass("Tied object survived exiting threads");
f47bb1
+
f47bb1
+undef $db;
f47bb1
+untie %h;
f47bb1
+unlink <threads*>;
f47bb1
-- 
f47bb1
2.5.5
f47bb1