Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segmentation fault when using Storable on DB_File #12626

Closed
p5pRT opened this issue Dec 3, 2012 · 14 comments
Closed

Segmentation fault when using Storable on DB_File #12626

p5pRT opened this issue Dec 3, 2012 · 14 comments

Comments

@p5pRT
Copy link

p5pRT commented Dec 3, 2012

Migrated from rt.perl.org#115972 (status was 'resolved')

Searchable as RT115972$

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2012

From @fangly

This is a bug report for perl from florent.angly@​gmail.com,
generated with the help of perlbug 1.39 running under perl 5.14.2.


Hi,

From the acknowledgments and the test suite of Storable, it appears
that this module should support serialization and deserialization of
tied hashes. However, I encountered a segmentation fault when trying to
freeze and thaw DB_File tied hashes using Storable.

This code reproduces the issue​:
  use strict;
  use warnings;
  use DB_File;
  use Storable qw(freeze thaw);
  tie my %offsets, 'DB_File', 'file.idx', O_CREAT|O_RDWR, 0644 or die
"Problem​: $!";
  $offsets{4} = 235;
  my $thing = thaw freeze \%offsets;

Note that the bug is reproducible when using GDBM_File instead of
DB_File. Note also, that the segmentation fault is triggered by the
thaw() statement, but happens at the very end of the script, during
garbage collection.

I generated a stack trace for this error​:
  (gdb) bt
  #0 0x00007ffff73b7118 in main_arena () from
/lib/x86_64-linux-gnu/libc.so.6
  #1 0x00007ffff652fe68 in __db_errcall () from
/usr/lib/x86_64-linux-gnu/libdb-5.1.so
  #2 0x00007ffff653013e in __db_errx () from
/usr/lib/x86_64-linux-gnu/libdb-5.1.so
  #3 0x00007ffff6533911 in __dbc_close_pp () from
/usr/lib/x86_64-linux-gnu/libdb-5.1.so
  #4 0x00007ffff67d9345 in XS_DB_File_DESTROY (my_perl=0x603010,
cv=<optimized out>) at DB_File.c​:1749
  #5 0x00007ffff7b155ac in Perl_pp_entersub (my_perl=0x603010) at
pp_hot.c​:3046
  #6 0x00007ffff7aa8a11 in Perl_call_sv
(my_perl=my_perl@​entry=0x603010, sv=sv@​entry=0x78bf80,
flags=flags@​entry=45) at perl.c​:2647
  #7 0x00007ffff7b1ba79 in S_curse (check_refcnt=1 '\001',
sv=0x78a460, my_perl=0x603010) at sv.c​:6342
  #8 Perl_sv_clear (my_perl=my_perl@​entry=0x603010,
orig_sv=orig_sv@​entry=0x623ca8) at sv.c​:6073
  #9 0x00007ffff7b1c132 in Perl_sv_free2
(my_perl=my_perl@​entry=0x603010, sv=0x623ca8) at sv.c​:6474
  #10 0x00007ffff7af6808 in S_mg_free_struct (my_perl=0x603010,
sv=<optimized out>, mg=0x640b80) at mg.c​:566
  #11 0x00007ffff7af736e in Perl_mg_free
(my_perl=my_perl@​entry=0x603010, sv=sv@​entry=0x6245c0) at mg.c​:588
  #12 0x00007ffff7b1b70a in Perl_sv_clear
(my_perl=my_perl@​entry=0x603010, orig_sv=orig_sv@​entry=0x606b78) at
sv.c​:6085
  #13 0x00007ffff7b1c132 in Perl_sv_free2
(my_perl=my_perl@​entry=0x603010, sv=0x606b78) at sv.c​:6474
  #14 0x00007ffff7b41920 in Perl_free_tmps
(my_perl=my_perl@​entry=0x603010) at scope.c​:167
  #15 0x00007ffff7aae574 in perl_run (my_perl=0x603010) at perl.c​:2273
  #16 0x0000000000400f89 in main (argc=2, argv=0x7fffffffe818,
env=0x7fffffffe830) at perlmain.c​:120

Best,

Florent



Flags​:
  category=library
  severity=low
  module=Storable


Site configuration information for perl 5.14.2​:

Configured by Debian Project at Sun Nov 4 10​:59​:22 UTC 2012.

Summary of my perl5 (revision 5 version 14 subversion 2) configuration​:

  Platform​:
  osname=linux, osvers=3.2.0-4-amd64,
archname=x86_64-linux-gnu-thread-multi
  uname='linux madeleine 3.2.0-4-amd64 #1 smp debian 3.2.32-1 x86_64
gnulinux '
  config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN
-D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector --param=ssp-buffer-size=4
-Wformat -Werror=format-security -Dldflags= -Wl,-z,relro
-Dlddlflags=-shared -Wl,-z,relro -Dcccdlflags=-fPIC
-Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.14
-Darchlib=/usr/lib/perl/5.14 -Dvendorprefix=/usr
-Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5
-Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.14.2
-Dsitearch=/usr/local/lib/perl/5.14.2 -Dman1dir=/usr/share/man/man1
-Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1
-Dsiteman3dir=/usr/local/man/man3 -Duse64bitint -Dman1ext=1
-Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm
-Uusesfio -Uusenm -Ui_libutil -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib
-Dlibperl=libperl.so.5.14.2 -des'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=define, usemultiplicity=define
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN
-fstack-protector -fno-strict-aliasing -pipe -I/usr/local/include
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2 -g',
  cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fstack-protector
-fno-strict-aliasing -pipe -I/usr/local/include'
  ccversion='', gccversion='4.7.2', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib
/usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib
  libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
  perllibs=-ldl -lm -lpthread -lc -lcrypt
  libc=, so=so, useshrplib=true, libperl=libperl.so.5.14.2
  gnulibc_version='2.13'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib
-fstack-protector'

Locally applied patches​:


@​INC for perl 5.14.2​:
  /etc/perl
  /usr/local/lib/perl/5.14.2
  /usr/local/share/perl/5.14.2
  /usr/lib/perl5
  /usr/share/perl5
  /usr/lib/perl/5.14
  /usr/share/perl/5.14
  /usr/local/lib/site_perl
  .


Environment for perl 5.14.2​:
  HOME=/home/floflooo
  LANG=en_AU.utf8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/usr/local/bin​:/usr/bin​:/bin​:/usr/local/games​:/usr/games
  PERL_BADLANG (unset)
  SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2012

From @Leont

On Sun Dec 02 17​:49​:11 2012, fangly wrote​:

From the acknowledgments and the test suite of Storable, it appears
that this module should support serialization and deserialization of
tied hashes. However, I encountered a segmentation fault when trying to
freeze and thaw DB_File tied hashes using Storable.

The problem isn't ties, it's that the DB_File object is implemented in C
and thus can't be serialized easily.

Note that the bug is reproducible when using GDBM_File instead of
DB_File. Note also, that the segmentation fault is triggered by the
thaw() statement, but happens at the very end of the script, during
garbage collection.

I bet it's reproducible on any class using T_PTROBJ. When the Perl
object is serialized, it copies the *pointer* to the C object, not the
object itself, so when it is deserialized it you end up with two perl
objects pointing to the same C object. When the perl objects are
destroyed, the C object will be destroyed twice, resulting in a
segfault. Something similar happens after thread-cloning AFAIK. I'm not
sure this can be fixed sanely (it would require either cloning the BDB
object, or refcounting and locking it), possibly it should die on
freezing instead of segfaulting much later.

Leon

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2012

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2012

From @bulk88

On Mon Dec 03 18​:03​:35 2012, LeonT wrote​:

I bet it's reproducible on any class using T_PTROBJ. When the Perl
object is serialized, it copies the *pointer* to the C object, not the
object itself, so when it is deserialized it you end up with two perl
objects pointing to the same C object. When the perl objects are
destroyed, the C object will be destroyed twice, resulting in a
segfault. Something similar happens after thread-cloning AFAIK. I'm not
sure this can be fixed sanely (it would require either cloning the BDB
object, or refcounting and locking it), possibly it should die on
freezing instead of segfaulting much later.

Leon

The rest of this post has nothing specific about DB_File.

The problem you describe isn't very difficult to fix. I have an
implementation of it at

https://metacpan.org/source/BULKDD/Win32-API-0.74/Callback/Callback.xs#L624
https://metacpan.org/source/BULKDD/Win32-API-0.74/Callback/Callback.xs#L656
https://metacpan.org/source/BULKDD/Win32-API-0.74/Callback/Callback.xs#L294

It adds a refcount of the number of interps/SVs that own a malloc block.
Last :​:DESTROY or whichever :​:DESTROY wins the refcnt=0 race frees it.
It uses Win32 atomic operations. The refcount is stuck onto the end of
the HeapAlloc block, with alignment padding if required. A pointer to
the refcount is stored in the mg struct. There is no realloc style
operation in my API and its write once (at creation, not shown in links
above), read many, so locks/synchronization is only required for the
freeing/DESTROY aspect.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Mar 6, 2013

From @fangly

The problem you describe isn't very difficult to fix. I have an
implementation of it

Thanks for the reply bulk88, but do you suggest that my code could be
changed to make it work? Or did you mean that a fix could be made for
Storable? If that's the case, would you be able to fix Storable?
Thanks,
Florent

@p5pRT
Copy link
Author

p5pRT commented Mar 6, 2013

From @bulk88

On Tue Mar 05 17​:25​:24 2013, fangly wrote​:

The problem you describe isn't very difficult to fix. I have an
implementation of it

Thanks for the reply bulk88, but do you suggest that my code could be
changed to make it work? Or did you mean that a fix could be made for
Storable? If that's the case, would you be able to fix Storable?
Thanks,
Florent

I was speaking generically about the problem LeonT described with Perl,
XS, and C libraries, I don't know anything specific about DB_File.
Storable specifically has an API
http​://search.cpan.org/~ams/Storable-2.39/Storable.pm#Hooks for classes
to define subs so they can de/serialize themselves correctly, often with
XSUBs. The simplest fix would be to die() to happen a hypothetical
DB_File​::STORABLE_freeze() and this would turn a SEGV to a friendly
fatal error ( see example
https://metacpan.org/source/FLORA/perl-5.17.4/cpan/Compress-Raw-Bzip2/lib/Compress/Raw/Bzip2.pm#L93
). A similar thing is often done for psuedo and/or fork see
http​://perldoc.perl.org/perlmod.html#CLONE_SKIP . The more complicated
but correct fix would be to implement a STORABLE_freeze() and do all the
correct C-land fixups on the tied var/obj.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Mar 6, 2013

From @bulk88

On Tue Mar 05 17​:44​:04 2013, bulk88 wrote​:

On Tue Mar 05 17​:25​:24 2013, fangly wrote​:

Thanks for the reply bulk88, but do you suggest that my code could
be
changed to make it work? Or did you mean that a fix could be made
for
Storable? If that's the case, would you be able to fix Storable?
Thanks,
Florent

I was speaking generically about the problem LeonT described with

If the solution is implementing Storable hooks in DB_File, then I think
this ticket needs to be moved to DB_File's CPAN RT Queue for someone
familiar (AKA authors/maintainers) with DB_File to respond since there
were no answers specific to DB_File so far (3 months).

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Mar 6, 2013

From @pmqs

Sorry for not responding to this sooner - just happened to notice this
thread pop up on p5p. If it was on the CPAN queue I would have spotted
it immediately.

I will (at least) get DB_File to do the same as Compress​::Raw​::Bzip2
(another of my modules) and die gracefully.

Florent - I'm curious about why you need Storable for DB_File at all.
DB_File already persists Perl hashes & array to disk, so I'm not clear
what extra Storable support gives us.

cheers

Paul

@p5pRT
Copy link
Author

p5pRT commented Mar 9, 2013

From @pmqs

Turns out DB_File can handle freeze/thaw gracefully since version
1.823. Perl 1.14.2 only has DB_File 1.821

Paul

@p5pRT
Copy link
Author

p5pRT commented Mar 9, 2013

@iabyn - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this as completed Mar 9, 2013
@p5pRT
Copy link
Author

p5pRT commented Mar 11, 2013

From @fangly

On 06/03/13 20​:45, Paul Marquess via RT wrote​:

Florent - I'm curious about why you need Storable for DB_File at all.
DB_File already persists Perl hashes & array to disk, so I'm not clear
what extra Storable support gives us.
Paul, the reason I need Storable for DB_File is that I want to use some
objects with Threads​::Queue​::Any, which rely of Storable to do its job.
The objects do not inherit from DB_File itself, but they do keep a
record of the DB_File object they need to work.

On 09/03/13 22​:44, Paul Marquess via RT wrote​:

Turns out DB_File can handle freeze/thaw gracefully since version
1.823. Perl 1.14.2 only has DB_File 1.821

That's good that DB_File now dies instead of segfaulting.

However, the reason I originally posted this report on the Perl5 tracker
is that the segmentation fault is not specific to DB_File. It also
happens with GDBM_File, and possibly other such modules​:

use strict;
use warnings;
use GDBM_File;
use Storable qw(freeze thaw);
tie my %offsets, 'GDBM_File', 'file.idx', &GDBM_WRCREAT, 0640 or die
"Problem​: $!\n";
$offsets{4} = 235;
my $thing = thaw freeze \%offsets;

Should all XXX_File modules implement hooks for Storable (as mentioned
by bulk88)?

Or is it that Storable could be smarter about dealing with XXX_File
objects? What do the Storable maintainers think?

Florent

@p5pRT
Copy link
Author

p5pRT commented Mar 11, 2013

From @lizmat

On Mar 11, 2013, at 3​:39 AM, Florent Angly <florent.angly@​gmail.com> wrote​:

On 06/03/13 20​:45, Paul Marquess via RT wrote​:

Florent - I'm curious about why you need Storable for DB_File at all.
DB_File already persists Perl hashes & array to disk, so I'm not clear
what extra Storable support gives us.
Paul, the reason I need Storable for DB_File is that I want to use some objects with Threads​::Queue​::Any, which rely of Storable to do its job. The objects do not inherit from DB_File itself, but they do keep a record of the DB_File object they need to work.

From the pod of Thread​::Queue​::Any​:

# specify class with "freeze" and "thaw" methods
use Thread​::Queue​::Any serializer => 'Storable';

So maybe another serializer might do the job for you? Maybe Sereal?

Liz

@p5pRT
Copy link
Author

p5pRT commented Mar 11, 2013

From @tsee

On 03/11/2013 03​:39 AM, Florent Angly wrote​:

On 06/03/13 20​:45, Paul Marquess via RT wrote​:

Florent - I'm curious about why you need Storable for DB_File at all.
DB_File already persists Perl hashes & array to disk, so I'm not clear
what extra Storable support gives us.
Paul, the reason I need Storable for DB_File is that I want to use some
objects with Threads​::Queue​::Any, which rely of Storable to do its job.
The objects do not inherit from DB_File itself, but they do keep a
record of the DB_File object they need to work.

However, the reason I originally posted this report on the Perl5 tracker
is that the segmentation fault is not specific to DB_File. It also
happens with GDBM_File, and possibly other such modules​:

Should all XXX_File modules implement hooks for Storable (as mentioned
by bulk88)?

Or is it that Storable could be smarter about dealing with XXX_File
objects? What do the Storable maintainers think?

FWIW, I think there is no "Storable maintainer" outside the general
"supported by perl5-porters".

I think Storable should UNDER NO CIRCUMSTANCES add support for specific
other modules[1]. The XXX_File modules could add hooks OR this could be
a case of "don't do that" since serializing those types of object is at
least a very shady thing to begin with.

--Steffen

[1] Without looking at the Storable code again, I bet that there's a
bunch of crazy exceptions in there already, but that should serve as an
example of what NOT to do.

@p5pRT
Copy link
Author

p5pRT commented Mar 25, 2013

From @fangly

Thanks for the answers. The best seem to have the different XX_File
modules have hooks for Storable.

I have filed a separate bug report for DB_File at
https://rt.cpan.org/Ticket/Display.html?id=84184&results=8cc77d5768d33c8b8c3aebdaffd39622

I have also confirmed that GDBM_File, NDBM_File, ODBM_File and SDBM_File
exhibit the same issue. Since these modules are maintained and shipped
with perl, should the present bug report be updated or a new one started?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant