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