You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucy.apache.org by Marvin Humphrey <ma...@rectangular.com> on 2011/11/09 02:32:46 UTC
[lucy-dev] Non-deterministic destruction in Perl 5.15
Greets,
In Perl 5.15 (current "blead" Perl -- the developer release), Lucy fails most
of its tests because of an exception thrown during global destruction:
(in cleanup) Insane attempt to destroy VTable for class 'Lucy::Object::Obj'
lucy_VTable_destroy at /home/sts/cpansmoke/perl-5.15.2/cpan/build/Lucy-0.2.2-o_YHcb/core/Lucy/Object/VTable.c line 44
at t/018-host.t line 0
That's a tripwire that I set because VTable's destructor should *never* be
invoked. We leak VTables on purpose.
What has changed in Perl 5.15 is that destructors are now called during global
destruction; previously, Perl freed all SVs during global destruction but did
not call DESTROY on objects.
http://search.cpan.org/~stevan/perl-5.15.3/pod/perlobj.pod#Global_Destruction
This change to Perl is going to require a corresponding change
to Lucy's Perl bindings. Consider the following code:
my %hash = (
searcher => Lucy::Search::IndexSearcher->new(index => $path),
);
$hash{circular_reference} = \%hash;
Because of the circular reference, that Perl hash, the Searcher it refers to,
and crucially, the Searcher's inner PolyReader will not be deallocated until
global destruction. During global destruction, though, refcounting goes out
the window and destruction order is effectively random.
What we would ordinarily want to see is destruction moving from the outermost
object to the innermost:
Perl hash
IndexSearcher
PolyReader
SegReaders
DataReaders
InStreams
FileHandles
...
This is important because when we get to the IndexSearcher's destructor, its
subcomponents still need to be valid:
void
IxSearcher_destroy(IndexSearcher *self) {
DECREF(self->reader);
// ...
SUPER_DESTROY(self, INDEXSEARCHER);
}
If self->reader has already been freed when this destructor gets called,
that's bad news -- we're going to be invoking DECREF on freed memory.
As far as I can tell, the only solution is to disconnect our DESTROY methods
when Perl enters global destruction and leak everything. Here's sample XS
code to get the point across:
void
DESTROY(self)
lucy_IndexSearcher *self;
PPCODE:
if (PL_phase != PERL_PHASE_DESTRUCT) {
lucy_IxSearcher_destroy(self);
}
Of course, this defeats the purpose of the change that was made in Perl 5.15.
The rationale for the new behavior is to support situations where for example,
you could guarantee that when a Perl interpreter in an embedded system shuts
down, *everything* gets reclaimed. But I believe that architecture is only
feasible when you control all memory allocation (as when the OS closes a
process) and thus Perl's new global destruction model is flawed as it cannot
encompass external resources.
I wonder how many other systems like ours are out there that are vulnerable to
this flaw. Not many CPAN distros are going to have test suites that validate
behavior under refcount leakage.
Marvin Humphrey
Re: [lucy-dev] Non-deterministic destruction in Perl 5.15
Posted by bc...@methodlogic.net.
On Tue, Nov 08, 2011 at 05:32:46PM -0800, Marvin Humphrey wrote:
> Greets,
Is this behaviour tuneable in anyway, or just the New Standard, period?
> In Perl 5.15 (current "blead" Perl -- the developer release), Lucy fails most
> of its tests because of an exception thrown during global destruction:
>
> (in cleanup) Insane attempt to destroy VTable for class 'Lucy::Object::Obj'
> lucy_VTable_destroy at /home/sts/cpansmoke/perl-5.15.2/cpan/build/Lucy-0.2.2-o_YHcb/core/Lucy/Object/VTable.c line 44
> at t/018-host.t line 0
>
> That's a tripwire that I set because VTable's destructor should *never* be
> invoked. We leak VTables on purpose.
>
> What has changed in Perl 5.15 is that destructors are now called during global
> destruction; previously, Perl freed all SVs during global destruction but did
> not call DESTROY on objects.
>
> http://search.cpan.org/~stevan/perl-5.15.3/pod/perlobj.pod#Global_Destruction
>
> This change to Perl is going to require a corresponding change
> to Lucy's Perl bindings. Consider the following code:
>
> my %hash = (
> searcher => Lucy::Search::IndexSearcher->new(index => $path),
> );
> $hash{circular_reference} = \%hash;
>
> Because of the circular reference, that Perl hash, the Searcher it refers to,
> and crucially, the Searcher's inner PolyReader will not be deallocated until
> global destruction. During global destruction, though, refcounting goes out
> the window and destruction order is effectively random.
>
> What we would ordinarily want to see is destruction moving from the outermost
> object to the innermost:
>
> Perl hash
> IndexSearcher
> PolyReader
> SegReaders
> DataReaders
> InStreams
> FileHandles
> ...
>
> This is important because when we get to the IndexSearcher's destructor, its
> subcomponents still need to be valid:
>
> void
> IxSearcher_destroy(IndexSearcher *self) {
> DECREF(self->reader);
> // ...
> SUPER_DESTROY(self, INDEXSEARCHER);
> }
>
> If self->reader has already been freed when this destructor gets called,
> that's bad news -- we're going to be invoking DECREF on freed memory.
>
> As far as I can tell, the only solution is to disconnect our DESTROY methods
> when Perl enters global destruction and leak everything. Here's sample XS
> code to get the point across:
>
> void
> DESTROY(self)
> lucy_IndexSearcher *self;
> PPCODE:
> if (PL_phase != PERL_PHASE_DESTRUCT) {
> lucy_IxSearcher_destroy(self);
> }
>
> Of course, this defeats the purpose of the change that was made in Perl 5.15.
> The rationale for the new behavior is to support situations where for example,
> you could guarantee that when a Perl interpreter in an embedded system shuts
> down, *everything* gets reclaimed. But I believe that architecture is only
> feasible when you control all memory allocation (as when the OS closes a
> process) and thus Perl's new global destruction model is flawed as it cannot
> encompass external resources.
>
> I wonder how many other systems like ours are out there that are vulnerable to
> this flaw. Not many CPAN distros are going to have test suites that validate
> behavior under refcount leakage.
>
> Marvin Humphrey
>
--
Brad Harder
Method Logic Digital Consulting
http://methodlogic.net/
http://twitter.com/bcharder
Re: [lucy-dev] Non-deterministic destruction in Perl 5.15
Posted by Marvin Humphrey <ma...@rectangular.com>.
Hi,
I'm going to reply to the stuff that I know best first, then go study and
reply to the rest later.
On Fri, Nov 11, 2011 at 02:51:26PM -0800, webmasters@ctosonline.org wrote:
> > void
> > IxSearcher_destroy(IndexSearcher *self) {
> > DECREF(self->reader);
>
> This seems to answer my question in the negative.
>
> From reading this code superficially, it looks as though the Searcher object
> has an internal (non-Perl) reference count on the reader. The Perl object
> will also have a reference count on the reader. That should prevent the
> reader from being destroyed before the searcher is.
Lucy objects gain and cache a Perl object (in self->ref.host_obj) as soon as
they are accessed from Perl-space, or as soon as their refcount reaches 4.
Once that happens, the Lucy object and its cached Perl object are linked for
life, and the Lucy object defers to the Perl object's refcount from that point
forward[1].
So that it's entirely possible that DECREF(self->reader) will end up invoking
SvREFCNT_dec() on self->reader->ref.host_obj.
> But you do seem to be implying that VTables need to be present for anything
> to work.
That's right.
This code...
DECREF(self->reader);
... means "If self->reader is not NULL, invoke its RefCount_Dec() method".
Since Lucy's method dispatch mechanism is vtable-based, it would be bad if
self->reader's VTable member (located at self->reader->vtable) gets destroyed
before we invoke RefCount_Dec() on self->reader.
But really, VTables should never be destroyed at all. The VTable objects for
all compiled classes in Lucy are allocated from static memory at compile-time
-- you really don't want to free() them! Hence this tripwire in VTable.c,
which is the superficial cause of all of our Perl 5.15 CPAN Testers failures:
void
VTable_destroy(VTable *self) {
THROW(ERR, "Insane attempt to destroy VTable for class '%o'", self->name);
}
Marvin Humphrey
[1] Refcounting in Lucy is method-based, so some classes override this.
Re: [lucy-dev] Non-deterministic destruction in Perl 5.15
Posted by we...@ctosonline.org.
On Dec 8, 2011, at 7:05 PM, Marvin Humphrey wrote:
> Thank you for sharing your valuable expertise on this subject.
:-)
A couple of years ago it was you who were explaining to me how overloading works. It’s funny how things have turned out.
Re: [lucy-dev] Non-deterministic destruction in Perl 5.15
Posted by Marvin Humphrey <ma...@rectangular.com>.
On Fri, Nov 11, 2011 at 02:51:26PM -0800, webmasters@ctosonline.org wrote:
> Perl previously did call DESTROY on objects during global destruction, and
> the order was non-determistic, but a few objects would escape the purge, in
> particular:
>
> • blessed array elements (bless \$_[0])
> • blessed closure variables (bless my \$x; sub foo { $x ... })
> • any other unreferenced SVs (not referenced by RVs or GVs)
>
> The VTables belong to the third category.
This is the key.
> Let’s look at the relevant code from the perl source:
>
> > void
> > Perl_sv_clean_objs(pTHX)
> > {
> > dVAR;
> > GV *olddef, *olderr;
> > PL_in_clean_objs = TRUE;
>
> This line goes through all scalars that are references to objects and calls
> undef() on them:
>
> > visit(do_clean_objs, SVf_ROK, SVf_ROK);
> The next two function calls eliminate all blessed GV slots. I think the GV
> slots are nulled and the SVs in them have their reference count lowered, but
> I haven’t actually read the code.
>
> > /* Some barnacles may yet remain, clinging to typeglobs.
> > * Run the non-IO destructors first: they may want to output
> > * error messages, close files etc */
> > visit(do_clean_named_objs, SVt_PVGV|SVpgv_GP, SVTYPEMASK|SVp_POK|SVpgv_GP);
> > visit(do_clean_named_io_objs, SVt_PVGV|SVpgv_GP, SVTYPEMASK|SVp_POK|SVpgv_GP);
>
> This is the bit added in 5.15. It looks for any objects remaining. Since
> they may be referenced by other objects (indirectly, through closures or
> array elements), whose destructors have not fired yet, they are not actually
> freed, but simply cursed; that is, they revert to non-object status
> (something you cannot do from Perl or XS, even though the core has the
> facility to do it).
OK, by now every Lucy object created by a direct user action should have gone
away, even if it was stuck in e.g. a Perl hash circular reference.
> > /* And if there are some very tenacious barnacles clinging to arrays,
> > closures, or what have you.... */
> > visit(do_curse, SVs_OBJECT, SVs_OBJECT);
The only Lucy objects remaining at this point should be:
* Static objects such as the VTables for all compiled classes.
* Dynamically created VTables for Perl-space subclasses.
* The VTable_registry (which is a Lucy::Object::LockFreeRegistry).
* The keys for the VTable_registry, which are all CharBufs.
After examining the relevant destructors, I am satisfied that it doesn't
matter in which order any cached Perl objects associated with those Lucy
objects get reclaimed, so long as we sever the connection between the
Perl-space DESTROY method for Lucy::Object::VTable and VTable_destroy().
> So based on that it looks as though you simply need to remove the destructor
> on VTables, since they will be destroyed last. Or create a destructor that
> makes sure all other Lucy objects have been purged.
>
> Now I hope I have you thoroughly confused. :-)
It took me a while to understand, but you have persuaded me that your solution
is the right one.
Thank you for sharing your valuable expertise on this subject.
Marvin Humphrey
Re: [lucy-dev] Non-deterministic destruction in Perl 5.15
Posted by we...@ctosonline.org.
On Nov 8, 2011, at 5:32 PM, Marvin Humphrey wrote:
> Greets,
>
> In Perl 5.15 (current "blead" Perl -- the developer release), Lucy fails most
> of its tests because of an exception thrown during global destruction:
>
> (in cleanup) Insane attempt to destroy VTable for class 'Lucy::Object::Obj'
> lucy_VTable_destroy at /home/sts/cpansmoke/perl-5.15.2/cpan/build/Lucy-0.2.2-o_YHcb/core/Lucy/Object/VTable.c line 44
> at t/018-host.t line 0
>
> That's a tripwire that I set because VTable's destructor should *never* be
> invoked. We leak VTables on purpose.
>
> What has changed in Perl 5.15 is that destructors are now called during global
> destruction; previously, Perl freed all SVs during global destruction but did
> not call DESTROY on objects.
Perl previously did call DESTROY on objects during global destruction, and the order was non-determistic, but a few objects would escape the purge, in particular:
• blessed array elements (bless \$_[0])
• blessed closure variables (bless my \$x; sub foo { $x ... })
• any other unreferenced SVs (not referenced by RVs or GVs)
The VTables belong to the third category.
>
> http://search.cpan.org/~stevan/perl-5.15.3/pod/perlobj.pod#Global_Destruction
>
> This change to Perl is going to require a corresponding change
> to Lucy's Perl bindings. Consider the following code:
>
> my %hash = (
> searcher => Lucy::Search::IndexSearcher->new(index => $path),
> );
> $hash{circular_reference} = \%hash;
>
> Because of the circular reference, that Perl hash, the Searcher it refers to,
> and crucially, the Searcher's inner PolyReader will not be deallocated until
> global destruction. During global destruction, though, refcounting goes out
> the window and destruction order is effectively random.
How has Lucy worked before, seeing that the order was already non-deterministic? Do they simply depend on the presence of the VTable?
>
> What we would ordinarily want to see is destruction moving from the outermost
> object to the innermost:
>
> Perl hash
> IndexSearcher
> PolyReader
> SegReaders
> DataReaders
> InStreams
> FileHandles
> ...
>
> This is important because when we get to the IndexSearcher's destructor, its
> subcomponents still need to be valid:
>
> void
> IxSearcher_destroy(IndexSearcher *self) {
> DECREF(self->reader);
This seems to answer my question in the negative.
From reading this code superficially, it looks as though the Searcher object has an internal (non-Perl) reference count on the reader. The Perl object will also have a reference count on the reader. That should prevent the reader from being destroyed before the searcher is.
> // ...
> SUPER_DESTROY(self, INDEXSEARCHER);
> }
>
> If self->reader has already been freed when this destructor gets called,
> that's bad news -- we're going to be invoking DECREF on freed memory.
>
> As far as I can tell, the only solution is to disconnect our DESTROY methods
> when Perl enters global destruction and leak everything. Here's sample XS
> code to get the point across:
>
> void
> DESTROY(self)
> lucy_IndexSearcher *self;
> PPCODE:
> if (PL_phase != PERL_PHASE_DESTRUCT) {
> lucy_IxSearcher_destroy(self);
> }
>
> Of course, this defeats the purpose of the change that was made in Perl 5.15.
> The rationale for the new behavior is to support situations where for example,
> you could guarantee that when a Perl interpreter in an embedded system shuts
> down, *everything* gets reclaimed. But I believe that architecture is only
> feasible when you control all memory allocation (as when the OS closes a
> process) and thus Perl's new global destruction model is flawed as it cannot
> encompass external resources.
Perl’s global destruction has always necessarily been flawed. It cannot but be non-deterministic, due to the way circular references work. There is simply no way to know which thing is the ‘outer’ object, and which is the ‘inner’, as they are all just linked, rather than ‘inner’ or ‘outer’.
I can’t say I fully understand why destroying the Perl-level Reader before the Searcher would be a problem. But you do seem to be implying that VTables need to be present for anything to work. If that is the case, then Lucy was already relying on an implementation detail, so why not continue to?
Let’s look at the relevant code from the perl source:
> void
> Perl_sv_clean_objs(pTHX)
> {
> dVAR;
> GV *olddef, *olderr;
> PL_in_clean_objs = TRUE;
This line goes through all scalars that are references to objects and calls undef() on them:
> visit(do_clean_objs, SVf_ROK, SVf_ROK);
The next two function calls eliminate all blessed GV slots. I think the GV slots are nulled and the SVs in them have their reference count lowered, but I haven’t actually read the code.
> /* Some barnacles may yet remain, clinging to typeglobs.
> * Run the non-IO destructors first: they may want to output
> * error messages, close files etc */
> visit(do_clean_named_objs, SVt_PVGV|SVpgv_GP, SVTYPEMASK|SVp_POK|SVpgv_GP);
> visit(do_clean_named_io_objs, SVt_PVGV|SVpgv_GP, SVTYPEMASK|SVp_POK|SVpgv_GP);
This is the bit added in 5.15. It looks for any objects remaining. Since they may be referenced by other objects (indirectly, through closures or array elements), whose destructors have not fired yet, they are not actually freed, but simply cursed; that is, they revert to non-object status (something you cannot do from Perl or XS, even though the core has the facility to do it).
> /* And if there are some very tenacious barnacles clinging to arrays,
> closures, or what have you.... */
> visit(do_curse, SVs_OBJECT, SVs_OBJECT);
> olddef = PL_defoutgv;
> PL_defoutgv = NULL; /* disable skip of PL_defoutgv */
> if (olddef && isGV_with_GP(olddef))
> do_clean_named_io_objs(aTHX_ MUTABLE_SV(olddef));
> olderr = PL_stderrgv;
> PL_stderrgv = NULL; /* disable skip of PL_stderrgv */
> if (olderr && isGV_with_GP(olderr))
> do_clean_named_io_objs(aTHX_ MUTABLE_SV(olderr));
> SvREFCNT_dec(olddef);
> PL_in_clean_objs = FALSE;
> }
So based on that it looks as though you simply need to remove the destructor on VTables, since they will be destroyed last. Or create a destructor that makes sure all other Lucy objects have been purged.
Now I hope I have you thoroughly confused. :-)
Re: [lucy-dev] Non-deterministic destruction in Perl 5.15
Posted by Peter Karman <pe...@peknet.com>.
Marvin Humphrey wrote on 11/08/2011 11:33 PM:
> It's not surprising that SWISH::3 would pass its tests. You haven't set up
> tests to validate behavior under refcount leakage, have you?
I do, actually.
http://cpansearch.perl.org/src/KARMAN/SWISH-3-1.000000/t/07-refcnt.t
among others.
I use a ref_cnt counter on the underlying C struct, not relying on the
Perl ref counting. So my DESTROY blocks are conditional on that ref_cnt.
void
DESTROY(self)
SV *self;
PREINIT:
swish_3 *s3;
CODE:
s3 = (swish_3*)sp_extract_ptr(self);
s3->ref_cnt--;
if (s3->ref_cnt < 1) {
sp_Stash_destroy( s3->stash );
}
but SWISH::3 is far less complex than Lucy in terms of the number of
classes and code generation etc, so I was willing to hand-code all the
ref counting etc of internal objects, etc.
>
> What I'm suggesting if a user writes a program which leaks Lucy objects, and
> if we disconnect that "tripwire" exception without implementing the
> PERL_PHASE_DESTRUCT wrapping, that program may segfault during global
> destruction in Perl 5.15.
>
> Most small programs don't leak, because most small programs don't have
> circular references. Circular refs are pretty common in big, complex
> programs, though.
>
> If SWISH::3 -- or Perl/Tk, or whatever else -- has complex objects which count
> on deterministic order of destruction, I believe that the potential for
> segfaulting during global destruction exists in Perl 5.15.
>
I agree with all that. I think your patch is the right choice.
--
Peter Karman . http://peknet.com/ . peter@peknet.com
Re: [lucy-dev] Non-deterministic destruction in Perl 5.15
Posted by Marvin Humphrey <ma...@rectangular.com>.
On Tue, Nov 08, 2011 at 07:53:03PM -0600, Peter Karman wrote:
> > void
> > DESTROY(self)
> > lucy_IndexSearcher *self;
> > PPCODE:
> > if (PL_phase != PERL_PHASE_DESTRUCT) {
> > lucy_IxSearcher_destroy(self);
> > }
> Would it help if Perl's global destruction phase called DESTROY() with an
> argument in order to indicate that it was in that phase? Or is that
> PERL_PHASE_DESTRUCT var supposed to achieve that? Is that real code?
That's real code. We autogenerate the method bindings, so the patch won't
look exactly like that, but PL_phase and PERL_PHASE_DESTRUCT are part of the
Perl C API in Perl 5.15. That construct will at least compile; I hope it
solves our problems.
> I guess what I'm getting at is, who cares if the change to Lucy defeats the
> purpose of the Perl feature.
I agree -- it's not important.
> The change to Perl is supposed to address a problem that Lucy does not have.
Well, Lucy wouldn't play well in an embedded system because we leak VTables.
We could fix that, but it's not a priority.
> Does putting that PERL_PHASE_DESTRUCT check in there work around the issue
> with no ill side effects to Lucy? Could we just plow through the code, add
> that check, and call it a day?
I've opened an issue and uploaded patches at
<https://issues.apache.org/jira/browse/LUCY-187>. As of now, I can get tests
passing on Perl 5.15, but test_valgrind still has lots of glitches. I can't
yet tell whether we're seeing long-standing problems that have suddenly
been revealed or new defects showing up.
> > I wonder how many other systems like ours are out there that are vulnerable to
> > this flaw. Not many CPAN distros are going to have test suites that validate
> > behavior under refcount leakage.
>
> fwiw, SWISH::3 is largely C/XS and it is apparently passing its 5.15 tests right
> now according to cpantesters. I don't have the same VTable architecture in
> there, but as you'll remember Marvin, the object and memory management model was
> inspired by our conversations around how Lucy does it.
It's not surprising that SWISH::3 would pass its tests. You haven't set up
tests to validate behavior under refcount leakage, have you?
What I'm suggesting if a user writes a program which leaks Lucy objects, and
if we disconnect that "tripwire" exception without implementing the
PERL_PHASE_DESTRUCT wrapping, that program may segfault during global
destruction in Perl 5.15.
Most small programs don't leak, because most small programs don't have
circular references. Circular refs are pretty common in big, complex
programs, though.
If SWISH::3 -- or Perl/Tk, or whatever else -- has complex objects which count
on deterministic order of destruction, I believe that the potential for
segfaulting during global destruction exists in Perl 5.15.
Marvin Humphrey
Re: [lucy-dev] Non-deterministic destruction in Perl 5.15
Posted by Peter Karman <pe...@peknet.com>.
Marvin Humphrey wrote on 11/8/11 7:32 PM:
> As far as I can tell, the only solution is to disconnect our DESTROY methods
> when Perl enters global destruction and leak everything. Here's sample XS
> code to get the point across:
>
> void
> DESTROY(self)
> lucy_IndexSearcher *self;
> PPCODE:
> if (PL_phase != PERL_PHASE_DESTRUCT) {
> lucy_IxSearcher_destroy(self);
> }
>
> Of course, this defeats the purpose of the change that was made in Perl 5.15.
> The rationale for the new behavior is to support situations where for example,
> you could guarantee that when a Perl interpreter in an embedded system shuts
> down, *everything* gets reclaimed. But I believe that architecture is only
> feasible when you control all memory allocation (as when the OS closes a
> process) and thus Perl's new global destruction model is flawed as it cannot
> encompass external resources.
>
Would it help if Perl's global destruction phase called DESTROY() with an
argument in order to indicate that it was in that phase? Or is that
PERL_PHASE_DESTRUCT var supposed to achieve that? Is that real code?
I guess what I'm getting at is, who cares if the change to Lucy defeats the
purpose of the Perl feature. The change to Perl is supposed to address a problem
that Lucy does not have. We already do Perl version checks where the API has
changed (as in regex handling). Does putting that PERL_PHASE_DESTRUCT check in
there work around the issue with no ill side effects to Lucy? Could we just plow
through the code, add that check, and call it a day?
> I wonder how many other systems like ours are out there that are vulnerable to
> this flaw. Not many CPAN distros are going to have test suites that validate
> behavior under refcount leakage.
fwiw, SWISH::3 is largely C/XS and it is apparently passing its 5.15 tests right
now according to cpantesters. I don't have the same VTable architecture in
there, but as you'll remember Marvin, the object and memory management model was
inspired by our conversations around how Lucy does it.
--
Peter Karman . http://peknet.com/ . peter@peknet.com