-
Notifications
You must be signed in to change notification settings - Fork 571
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
Storable -- segmentation fault #15125
Comments
From mthurn@cpan.orgCreated by mthurn@cpan.orgWhen my object contains a child JSON object, package Foo; package main; Perl Info
|
From @jkeenanOn Sun Jan 10 12:19:23 2016, mthurn@cpan.org wrote:
1. What is the version of Storable at which you experienced this failure? 2. Note that perl-5.10.1 was released in 2007 and is long out of support. The versions of Perl which are currently supported are 5.22 and 5.20. In addition, the version of Storable which accompanied the perl-5.10.1 was 2.20; the version of Storable released with perl-5.22.0 was 2.53. So both Perl and Storable have progressed considerably since then. 3. That being said, I was able to reproduce the segmentation fault with perl-5.22.0, Storable 2.53 and JSON 2.90. -- |
The RT System itself - Status changed from 'new' to 'open' |
From @shlomifOn Sun Jan 10 15:41:17 2016, jkeenan wrote:
And I was able to reproduce the segfault with bleadperl commit 5dcc841 and the latest JSON::MaybeXS and Cpanel::JSON::XS from CPAN: <<<<<<<<<<< package main; |
From @arcMartin Thurn <perlbug-followup@perl.org> wrote:
I think this needs JSON::XS or Cpanel::JSON::XS installed to trigger. use Cpanel::JSON::XS; But I don't think this is a bug that can be fixed in anything that p5p First, an instance of Cpanel::JSON::XS is created. Under the hood, the This instance is then frozen and re-thawed using Storable. Storable Then the thawed instance is discarded, so its destructor gets run. The I believe the only way to fix this is for Cpanel::JSON::XS and https://metacpan.org/pod/Storable#Hooks In addition, it's possible that it would be worth those modules being Inline Patchdiff --git i/typemap w/typemap
index 66d2b4a..e001065 100644
--- i/typemap
+++ w/typemap
@@ -10,6 +10,9 @@ T_JSON
&& (SvSTASH (SvRV ($arg)) == JSON_STASH || sv_derived_from
Regardless, I think these changes should be taken up with the https://github.com/rurban/Cpanel-JSON-XS/issues and the author of JSON::XS prefers reports to be emailed directly: https://metacpan.org/pod/JSON::XS#BUGS Thanks. -- |
@arc - Status changed from 'open' to 'rejected' |
From kingpin@dcswcc.orgThis is probably moot given the email traffic I've seen today, but FWIW - - Martin On Sun, Jan 10, 2016 at 6:41 PM, James E Keenan via RT <
|
From bkb@cpan.orgOn Mon Jan 11 10:28:38 2016, arc wrote:
It doesn't require JSON::XS or Cpanel::JSON::XS to trigger. Any XS module with a blessed object will do, this has nothing whatsoever to do with Cpanel::JSON::XS, JSON::XS, or any other module except Storable. For example: use JSON::Create;
This is called encapsulation of objects.
Storable breaks the encapsulation of the object, then it copies that into a new object, then it blesses the copy into the original class, and you're suggesting that the author of JSON::XS or Cpanel::JSON::XS should do something about that chain of events?
All of which is an exceedingly good example of why it's not a great idea to break object encapsulation of Perl objects, then bless copies of the fake object got by coping the broken encapsulation into the class of the original object. I don't know exactly what kind of structure is used by JSON::XS, but typically it would be a pointer into memory which is represented as an IV within Perl.
The authors of JSON::XS and Cpanel::JSON::XS, and any other XS module should not be forced to defensively program against this kind of misbehaviour by Storable. There is no possible way that a C programmer, given a pointer into random memory (the IV from Perl), can check that it is a valid instance of anything whatsoever, since that always involves accessing the memory, and there is no guarantee that even accessing the memory is possible.
Storable is the module which should be "in question" here. JSON::XS and Cpanel::JSON::XS are not misbehaving at all and do not need to be modified. The misbehaviour is to break object encapsulation and then bless the broken copy of the object into a class it doesn't belong to. |
From @bulk88I am not respond to any particular post in this ticket. There are 4 ways to store C resources (pointers or self-alloced-C structs) in Perl. Pointers only, in a SVIV #4 is rare, inside out objects with the pointer or C struct stored with one of the above methods in a global perl var, serializing the object is impossible with inside out objects because they are indexed by HV*s which change between perl process runs There is a number 5 way but such an object is insane. Pointers only, in the XSANY of a XSUB CV *, your object is reference to a blessed (X) subroutine, dtor is free magic attached to the CV *. I dont think Storable can deparse an XSUB and recreate it. -- |
From @arcBen Bullock via RT <perlbug-followup@perl.org> wrote:
I meant that the original reporter's test case yields no segfault in a
Well, I dare say there are XS modules with blessed objects to which
I'm suggesting that, if someone wants to be able to use Storable on
Your position that Storable breaks encapsulation by trying to dump and -- |
From bkb@cpan.orgOn Tue Jan 26 12:44:52 2016, arc wrote:
As bulk88 points out in his very informative post there are a number of different ways of making an object, but the method using IVs as described here: http://www.lemoda.net/perl/perl-xs-object/index.html will inevitably lead to errors of this kind. Unless you can demonstrate that the above method of blessing a T_PTROBJ is illegal somehow according to the Perl specifications it seems to me that Storable should not be doing what it is doing. It's possible for modules to defensively program against being sent bad objects, for example by containing a per-module hash table with random "safety" keys and values with the actual "dangerous" pointer values, then using the "safe" random keys as the data within the blessed objects, and checking every input object with the table and rejecting unknown keys. Then at least the segmentation fault error could be avoided. However, it's not clear to me why all XS module programmers should be forced to do this for the benefit of a module which breaks encapsulation and blesses a random pointer into the class without using the module's constructor. My opinion is that the user who wants to serialize those objects should be responsible for serializing them, something like this: https://metacpan.org/pod/distribution/JSON-Create/lib/JSON/Create.pod#obj
The part which causes the segmentation fault is blessing the copy of the object into the class JSON::XS within Storable rather than the breaking encapsulation part, so I would suggest either not blessing objects into classes or giving the user responsibility for object serialization as described above. The method I described at the top, of making "secret" objects hidden behind a hash of random keys could just as easily be applied to a pure Perl module which would break Storable in a similar way, although without the segmentation fault errors. |
From @arcMy sincere apologies for the very long delay in this reply. Ben Bullock via RT <perlbug-followup@perl.org> wrote:
For the record, I certainly don't think that T_PTROBJ and related
This could certainly be made to work. However, I don't think it's Storable does already call classes' STORABLE_freeze and STORABLE_thaw -- |
From @demerphqOn 24 April 2016 at 13:58, Aaron Crane <arc@cpan.org> wrote:
As a serialization author I would love a reliable way to tell if an For instance, imagine every namespace with XS code in it was Arguably this should be fixed by having a better way to create XS Yves -- |
From zefram@fysh.orgdemerphq wrote:
Already available: "magic" annotations can be attached to any Perl value, -zefram |
Migrated from rt.perl.org#127232 (status was 'rejected')
Searchable as RT127232$
The text was updated successfully, but these errors were encountered: