Proxy: (Fwd) [patch] make Storable thread-safe

From: Tim Bunce (Tim.Bunce_at_pobox.com)
Date: 01/19/04

Next message: Jo: "problems with DBI on HPUX 11i"

Previous message: Sean Kelly: "Re: Can't make DBD"
Next in thread: Stas Bekman: "Re: Proxy: (Fwd) [patch] make Storable thread-safe"
Reply: Stas Bekman: "Re: Proxy: (Fwd) [patch] make Storable thread-safe"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Date: Mon, 19 Jan 2004 12:23:23 +0000
To: dbi-users@perl.org

This is relevant to any brave people trying to use DBD::Proxy or
DBI::ProxyServer on unix in threaded mode or windows (where threads
are used to emulate forks).

Tim.

p.s. Thanks Stas!

----- Forwarded message from Stas Bekman <stas@stason.org> -----

Mailing-List: contact perl5-porters-help@perl.org; run by ezmlm
Date: Mon, 19 Jan 2004 00:20:02 -0800
From: Stas Bekman <stas@stason.org>
Organization: Hope, Humanized
To: The Perl5 Porters Mailing List <perl5-porters@perl.org>
Subject: [patch] make Storable thread-safe

As Jan Dubois has suggested a few months ago Storable 2.09 is not
thread-safe, because it doesn't handle PL_modglobal's Storable context entry
cloning on perl_clone. One of the windows users with mod_perl 2.0 has
already been beaten by this bug.

I've spent a lot of time trying to reproduce the problem on linux, but the
problem is this: All Storable user API functions are atomic context wise,
i.e. you can't run two Storable functions one after another so that they
will interfere with each other. So from the tester point of view I couldn't
figure out how to show that the problem is there. The problem will happen on
unix every time two threads will try to change the same context (since the
actual implementation is not thread-atomic, i.e. not serialized), not
talking about race conditions. It failed to come up with a reliable test
that will fail deterministically. If you have an idea how to come up with
such an 'exploit' I'm all ears.

Luckily on win32 we have the issue with private memory pool per thread and
it fails on win32 with a trivial test. "free to wrong pool" is the failure
message.

The following patch [inlined and attached] against Storable 2.09 makes it
thread-safe by allocating a new context for each new thread and adds a
simple test which will fail on Storable 2.09 on win32. Actually I haven't
tested it, since I don't have win32, so please verify that.

I'm not sure whether it's better to expose init_perinterp() to perl and have
CLONE on the perl side like I did, or just have CLONE defined in Storable.xs
like so:

void
CLONE()
CODE:
init_perinterp();

it's up to you to decide what's the best way.

Version 2.09
diff --new-file -ur Storable-2.09/MANIFEST Storable-2.10/MANIFEST
--- Storable-2.09/MANIFEST 2004-01-05 07:43:38.000000000 -0800
+++ Storable-2.10/MANIFEST 2004-01-18 23:48:52.000000000 -0800
@@ -29,6 +29,7 @@
t/tied.t See if Storable works
t/tied_hook.t See if Storable works
t/tied_items.t See if Storable works
+t/threads.t See if Storable works under ithreads
t/utf8.t See if Storable works
t/utf8hash.t See if Storable works
t/Test/Builder.pm For testing the CPAN release on pre 5.6.2
diff --new-file -ur Storable-2.09/Storable.pm Storable-2.10/Storable.pm
--- Storable-2.09/Storable.pm 2004-01-05 07:43:43.000000000 -0800
+++ Storable-2.10/Storable.pm 2004-01-15 17:39:00.000000000 -0800
@@ -21,7 +21,7 @@
use AutoLoader;
use vars qw($canonical $forgive_me $VERSION);

-$VERSION = '2.09';
+$VERSION = '2.10';
*AUTOLOAD = \&AutoLoader::AUTOLOAD; # Grrr...

#
@@ -47,6 +47,11 @@
}
}

+sub CLONE {
+ # clone context under threads
+ Storable::init_perinterp();
+}
+
# Can't Autoload cleanly as this clashes 8.3 with &retrieve
sub retrieve_fd { &fd_retrieve } # Backward compatibility

diff --new-file -ur Storable-2.09/Storable.xs Storable-2.10/Storable.xs
--- Storable-2.09/Storable.xs 2003-09-21 15:32:49.000000000 -0700
+++ Storable-2.10/Storable.xs 2004-01-15 17:29:22.000000000 -0800
@@ -5901,6 +5901,9 @@
gv_fetchpv("Storable::interwork_56_64bit", GV_ADDMULTI, SVt_PV);
#endif

+void
+init_perinterp()
+
int
pstore(f,obj)
OutputStream f
diff --new-file -ur Storable-2.09/t/threads.t Storable-2.10/t/threads.t
--- Storable-2.09/t/threads.t 1969-12-31 16:00:00.000000000 -0800
+++ Storable-2.10/t/threads.t 2004-01-18 23:59:23.000000000 -0800
@@ -0,0 +1,55 @@
+
+# as of 2.09 on win32 Storable w/threads dies with "free to wrong
+# pool" since it uses the same context for different threads. since
+# win32 perl implementation allocates a different memory pool for each
+# thread using the a memory pool from one thread to allocate memory
+# for another thread makes win32 perl very unhappy
+#
+# but the problem exists everywhere, not only on win32 perl , it's
+# just hard to catch it deterministically - since the same context is
+# used if two or more threads happen to change the state of the
+# context in the middle of the operation, and those operations aren't
+# atomic per thread, bad things including data loss and corrupted data
+# can happen.
+#
+# this has been solved in 2.10 by adding a Storable::CLONE which calls
+# Storable::init_perinterp() to create a new context for each new
+# thread when it starts
+
+sub BEGIN {
+ if ($ENV{PERL_CORE}){
+ chdir('t') if -d 't';
+ @INC = ('.', '../lib');
+ } else {
+ unshift @INC, 't';
+ }
+ require Config; import Config;
+ if ($ENV{PERL_CORE} and $Config{'extensions'} !~ /\bStorable\b/) {
+ print "1..0 # Skip: Storable was not built\n";
+ exit 0;
+ }
+ unless ($Config{'useithreads'} and eval { require threads; 1 }) {
+ print "1..0 # Skip: no threads\n";
+ exit 0;
+ }
+}
+
+use Test::More;
+
+use strict;
+
+use threads;
+use Storable qw(nfreeze);
+
+plan tests => 2;
+
+threads->new(\&sub1);
+
+$_->join() for threads->list();
+
+ok 1;
+
+sub sub1 {
+ nfreeze {};
+ ok 1;
+}

__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com

diff --new-file -ur Storable-2.09/ChangeLog Storable-2.10/ChangeLog
--- Storable-2.09/ChangeLog 2004-01-05 07:43:38.000000000 -0800
+++ Storable-2.10/ChangeLog 2004-01-19 00:04:02.000000000 -0800
@@ -1,3 +1,13 @@
+
+ Version 2.10
+
+ Add Storable::CLONE which calls Storable::init_perinterp() to
+ create a new context for each new perl ithread when it starts, to
+ make Storable thread-safe. add a test, which unfortunately fail
+ only on win32, as I wasn't able to come up with one failing on
+ unix even though the problem was there.
+ [Stas Bekman <stas@stason.org>, Jan Dubois <jand@ActiveState.com>]
+
Sat Jan 3 18:49:18 GMT 2004 Nicholas Clark <nick@ccl4.org>

     Version 2.09
diff --new-file -ur Storable-2.09/MANIFEST Storable-2.10/MANIFEST
--- Storable-2.09/MANIFEST 2004-01-05 07:43:38.000000000 -0800
+++ Storable-2.10/MANIFEST 2004-01-18 23:48:52.000000000 -0800
@@ -29,6 +29,7 @@
t/tied.t See if Storable works
t/tied_hook.t See if Storable works
t/tied_items.t See if Storable works
+t/threads.t See if Storable works under ithreads
t/utf8.t See if Storable works
t/utf8hash.t See if Storable works
t/Test/Builder.pm For testing the CPAN release on pre 5.6.2
diff --new-file -ur Storable-2.09/Storable.pm Storable-2.10/Storable.pm
--- Storable-2.09/Storable.pm 2004-01-05 07:43:43.000000000 -0800
+++ Storable-2.10/Storable.pm 2004-01-15 17:39:00.000000000 -0800
@@ -21,7 +21,7 @@
use AutoLoader;
use vars qw($canonical $forgive_me $VERSION);

-$VERSION = '2.09';
+$VERSION = '2.10';
*AUTOLOAD = \&AutoLoader::AUTOLOAD; # Grrr...

#
@@ -47,6 +47,11 @@
         }
}

+sub CLONE {
+ # clone context under threads
+ Storable::init_perinterp();
+}
+
# Can't Autoload cleanly as this clashes 8.3 with &retrieve
sub retrieve_fd { &fd_retrieve } # Backward compatibility

diff --new-file -ur Storable-2.09/Storable.xs Storable-2.10/Storable.xs
--- Storable-2.09/Storable.xs 2003-09-21 15:32:49.000000000 -0700
+++ Storable-2.10/Storable.xs 2004-01-15 17:29:22.000000000 -0800
@@ -5901,6 +5901,9 @@
     gv_fetchpv("Storable::interwork_56_64bit", GV_ADDMULTI, SVt_PV);
#endif

+void
+init_perinterp()
+
int
pstore(f,obj)
OutputStream f
diff --new-file -ur Storable-2.09/t/threads.t Storable-2.10/t/threads.t
--- Storable-2.09/t/threads.t 1969-12-31 16:00:00.000000000 -0800
+++ Storable-2.10/t/threads.t 2004-01-18 23:59:23.000000000 -0800
@@ -0,0 +1,55 @@
+
+# as of 2.09 on win32 Storable w/threads dies with "free to wrong
+# pool" since it uses the same context for different threads. since
+# win32 perl implementation allocates a different memory pool for each
+# thread using the a memory pool from one thread to allocate memory
+# for another thread makes win32 perl very unhappy
+#
+# but the problem exists everywhere, not only on win32 perl , it's
+# just hard to catch it deterministically - since the same context is
+# used if two or more threads happen to change the state of the
+# context in the middle of the operation, and those operations aren't
+# atomic per thread, bad things including data loss and corrupted data
+# can happen.
+#
+# this has been solved in 2.10 by adding a Storable::CLONE which calls
+# Storable::init_perinterp() to create a new context for each new
+# thread when it starts
+
+sub BEGIN {
+ if ($ENV{PERL_CORE}){
+ chdir('t') if -d 't';
+ @INC = ('.', '../lib');
+ } else {
+ unshift @INC, 't';
+ }
+ require Config; import Config;
+ if ($ENV{PERL_CORE} and $Config{'extensions'} !~ /\bStorable\b/) {
+ print "1..0 # Skip: Storable was not built\n";
+ exit 0;
+ }
+ unless ($Config{'useithreads'} and eval { require threads; 1 }) {
+ print "1..0 # Skip: no threads\n";
+ exit 0;
+ }
+}
+
+use Test::More;
+
+use strict;
+
+use threads;
+use Storable qw(nfreeze);
+
+plan tests => 2;
+
+threads->new(\&sub1);
+
+$_->join() for threads->list();
+
+ok 1;
+
+sub sub1 {
+ nfreeze {};
+ ok 1;
+}

----- End forwarded message -----

Next message: Jo: "problems with DBI on HPUX 11i"

Previous message: Sean Kelly: "Re: Can't make DBD"
Next in thread: Stas Bekman: "Re: Proxy: (Fwd) [patch] make Storable thread-safe"
Reply: Stas Bekman: "Re: Proxy: (Fwd) [patch] make Storable thread-safe"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Relevant Pages

Re: please answer these questions as soon as possible. because i have exam
... can the clone be identical to the original? ... If I have a client going against an RDBMS, the context is clear, it's ... representation in one context of employee 123456, ... process space, some other client program may also have such objects, ...
(comp.object)
Re: Why isnt shift the same as $_[0]?
... > returns a new object, not a clone etc, a new one. ... The "context" says if we ... - It ignores $obj altogether, ... I bet other behaviors have been implemented too. ...
(comp.lang.perl.misc)
Guess I have to give up trying to clone a menu!
... usercontrol and pass it to a form which merged it with the forms menu. ... both the context and mainmenu. ... A clone method for the menus would be great. ... updates work. ...
(microsoft.public.dotnet.languages.vb)
Re: Newbie: cloning a Number
... Philipp wrote: ... clone() itself is inherited from Object, ... Of course these objects are really Numbers, but the java-compiler ... In that context it seems safe to just use casts: ...
(comp.lang.java.programmer)
Re: How to implement a buffer (memory?) pool
... > I understand the general concept of a memory pool. ... > Allocating a buffer to instantiate objects doesn't seem to prevent the ... The fragmentation will now be localized to ... > specific to the context it is used. ...
(comp.lang.cpp)