[XS] DESTROY: free or flush? "related" objects?

[Ivan Shmakov]

BTW, is it common for the DESTROY method to flush any pending
writes associated with the object being destroyed?

The library I'm interested in offers two destructors for one of
its objects: one is "free", which simply deallocates the
object's memory, and the other is "close", which flushes the
pending writes first. It certainly feels more sensible to call
"close" in Perl's DESTROY method, but what do I do if it fails?
AIUI, calling die () in DESTROY is of little use.

The other issue is that given a library's object of type A,
I can request an object of type B (an iterator, in this case) to
be created. Obviously, as this latter object references the
former, I now have some refcounting trickery to do.

Alas, the library doesn't provide a way to retrieve the "parent"
object. Which makes me wonder, will it be sensible to represent
the B type as a (blessed) Perl reference to a list, holding the
pointers to both the "child" and "parent" objects? (The parent
object itself is represented as a reference to its pointer.)

Do I understand it correctly that I get refcounting for granted
in this case?

 

[Rainer Weikusat]

BTW, is it common for the DESTROY method to flush any pending
writes associated with the object being destroyed?

The library I'm interested in offers two destructors for one of
its objects: one is "free", which simply deallocates the
object's memory, and the other is "close", which flushes the
pending writes first. It certainly feels more sensible to call
"close" in Perl's DESTROY method, but what do I do if it
fails? AIUI, calling die () in DESTROY is of little use.

AFAIK, the only effect is that it possibly clobbers another exception,
cf

----------
package A;

sub new
{
return bless([], 'A');
}

sub DESTROY
{
die("In a destructor, no one hears you scream!");
}

package main;

eval {
my $a = A->new();

$a = undef;
print("I will survive!\n");
};

eval {
my $a = A->new();

die("After death I rot");
};

print("$@", "\n");
----------

My suggestion would be to call close and print a diagnostic
if it fails. That's consistent with the way 'other objects' work
(specifically, file handles) and better than calling the other:
There's a (usually good) chance that this will work as intended while
just freeing the object is equivalent to a close which always fails.

The other issue is that given a library's object of type A,
I can request an object of type B (an iterator, in this case) to
be created. Obviously, as this latter object references the
former, I now have some refcounting trickery to do.

Alas, the library doesn't provide a way to retrieve the "parent"
object. Which makes me wonder, will it be sensible to represent
the B type as a (blessed) Perl reference to a list, holding the
pointers to both the "child" and "parent" objects

Instead of a pointer to the parent object, I'd use a reference to the
corresponding Perl object. This would imply that the parent won't be
destroyed while an iterator referring to it still exists. Another
option would be to make that a weak reference. In this case, the
'parent member' of the iterator object would be cleared when the
parent object goes away which could be used for 'automatic iterator
invalidation'.

 

[Ivan Shmakov]

[...]

My suggestion would be to call close and print a diagnostic if it
fails.

How do I print the diagnostic, then? (I'm considering
implementing ->DESTROY () in the .xs code.)

That's consistent with the way 'other objects' work (specifically,
file handles) and better than calling the other: there's a (usually
good) chance that this will work as intended while just freeing the
object is equivalent to a close which always fails.

Sounds sensible; thanks.

Instead of a pointer to the parent object, I'd use a reference to the
corresponding Perl object. This would imply that the parent won't be
destroyed while an iterator referring to it still exists.

Indeed, that's what I've meant.

Another option would be to make that a weak reference. In this case,
the 'parent member' of the iterator object would be cleared when the
parent object goes away which could be used for 'automatic iterator
invalidation'.

This will require appropriate guards on all the XS methods
(which doesn't seem to be all that reasonable), and I'm not
entirely sure if freeing the parent before the child has, and
will have, well-defined semantics according to the library's
specification.

 

[Ivan Shmakov]

[...]

It certainly feels more sensible to call "close" in Perl's DESTROY
method, but what do I do if it fails? AIUI, calling die () in
DESTROY is of little use.

... Unless "use warnings" is in effect, as it seems to turn such
die () invocations into warn ().

AFAIK, the only effect is that it possibly clobbers another
exception, cf

I wonder if it still is the case in the newer Perl versions?
For instance, when run with $ perl -w, the code gives:

--cut--
(in cleanup) In a destructor, no one hears you scream! at - line 10.
(in cleanup) In a destructor, no one hears you scream! at - line 10.
I will survive!
After death I rot at - line 25.
--cut--

Or could that be specific to the Debian Perl package?

$ dpkg -s -- perl
Package: perl
....
Architecture: amd64
Version: 5.14.2-14
....
$

package A;

sub new
{
return bless([], 'A');
}

BTW, do I understand it correctly that in order to allow for
proper subclassing, it should generally be $_[0] // 'A' instead?

[...]

package main;

FWIW, I've tweaked this part of the code as follows.

use warnings;

eval {
my $a = A->new ();

$a = undef;
die ("I will survive!");
};

print STDERR ($@);

eval {
my $a = A->new ();

die ("Should clobber this?");
};

print STDERR ($@);

And the code now results in:

--cut--
(in cleanup) In a destructor, no one hears you scream! at - line 10.
I will survive! at - line 21.
(in cleanup) In a destructor, no one hears you scream! at - line 10.
Should clobber this? at - line 29.
--cut--

 

[Rainer Weikusat]

[...]

Sounds sensible; thanks.

I would say it's always worth doing any cleanup you can which will not
do any harm if it fails. If flushing might leave whatever its flushing
to in an inconsistent state if it fails, it would be better not to call
it from DESTROY and instead provide an explicit ->sync method of some
sort.

Actually, combining both is possible: Provide the explicit method for
people whose cultural background makes it difficult for them to
understand that 'automatic management' isn't necessarily restricted to
memory and for situations were 'checking the return value of close'
actually makes any sense (which it generally doesn't) and provide a
working destructor, ie, one which doesn't gratuitiously leave the
object in an inconsistent state because trying to make it consistent
could fail.

 

[Rainer Weikusat]

Ivan Shmakov <[email protected]> writes:

[...]

It certainly feels more sensible to call "close" in Perl's DESTROY
method, but what do I do if it fails? AIUI, calling die () in
DESTROY is of little use.

... Unless "use warnings" is in effect, as it seems to turn such
die () invocations into warn ().

AFAIK, the only effect is that it possibly clobbers another
exception, cf

I wonder if it still is the case in the newer Perl versions?

AFAIK, the behaviour changed with Perl 5.14 and generally 'for the better'.

[...]

package A;

sub new
{
return bless([], 'A');
}

BTW, do I understand it correctly that in order to allow for
proper subclassing, it should generally be $_[0] // 'A' instead?

In actual code, $_[0] should be used because this means the
constructor can be inherited.

 

[Ivan Shmakov]

Perl_warn, unless you want to offer some more sophisticated form of
diagnostic catching.

As die () in ->DESTROY () already seem to turn into a warn (),
if "use warnings" is in effect, I guess I'd stick to the former.
(But then, I wonder if Perl_croak () will so behave?)

There's another question, however: what's an easy way to
invalidate the object in an XS DESTROY ()? In Perl, I've seen
it being done as follows:

sub DESTROY {
## .
return
unless ($$self);

## ... clean the things up...

## .
$$self
= undef;
}

Now, I try to invalidate the object from within an XS function:

void
obj_free (obj)
XXX::Object obj;
CODE:
/* ... clean the things up... */

/* FIXME: accessing ST () directly */
sv_setref_pv (ST (0), 0, 0);

However, this appears to be subtly different in that (AIUI) it's
the reference that gets invalidated, and not just the referent
SV. (Thus, I cannot, say, XSRETURN_EMPTY early upon seeing such
a condition.)

Any suggestions on that?

TIA.

[...]

I would say it's always worth doing any cleanup you can which will
not do any harm if it fails. If flushing might leave whatever its
flushing to in an inconsistent state if it fails, it would be better
not to call it from DESTROY and instead provide an explicit ->sync
method of some sort.

To note is that it isn't all that obvious from the library's
documentation whether the state of the on-disk object being
manipulated will be consistent or not, should close () fail.
Somehow, I guess it won't.

So, I'm going to flush in DESTROY, yet advise in favor of
explicit flushing in the Perl interface's documentation.

OTOH, there isn't going to be a bunch of easy ways to recover
after a failure, whether it was detected or not.

[...]

If you're manipulating these objects from C, you don't need to use a
Perl array, nor a full Perl reference to the parent object. Just
create a little structure with an iterator* and a SV* in it, then
store that structure in the PV slot of the object (that is, not the
reference, the SV that reference points to).

Indeed, this makes it easier to access the objects from the
C side; and as for the Perl side, I'd only need ->parent (),
which could easily be implemented in XS. Thanks.

Put the iterator pointer in the iterator member, SvREFCNT_inc the
parent object (again, the object itself, not the reference pointing
to it), and put a pointer to it in the SV*. Then, in the iterator's
DESTROY method, free the iterator and SvREFCNT_dec the parent object
(and let perl handle cleaning up the structure, since it was the
'string value' of the object).

So, I ended up with roughly the following code.

SV *
new (class, parent)
SV *class
SV *parent
CODE:
/* ... should the arguments be sane... */ {
struct wrapped_iter *wr;
/* NB: class != 0, thus defclass = 0 won't do any harm */
RETVAL
= allocate_rv_pvn (sizeof (*wr), class, 0, &wr);
wr->parent
= SvREFCNT_inc (parent);
wr->iter
= iter;
}
OUTPUT:
RETVAL

void
iter_free (iter)
XXX::Iterator iter;
CODE:
SvREFCNT_dec (iter->parent);
iter_free (iter->iter);

/* NB: this one is similar to allocate_struct () in POSIX.xs */

static SV *
allocate_rv_pvn (const STRLEN size, SV *class, const char *defclass,
void **ptr)
{
SV *sv
= newSV (size);
/* turn this into a "valid" PV of the size given */
SvCUR_set (sv, size);
SvPOK_on (sv);

if (ptr) {
*ptr
= SvPV_nolen (sv);
}

/* . */
return
sv_bless (newRV_noinc (sv),
(class
? gv_stashsv (class, GV_ADD)
: gv_stashpv (defclass, GV_ADD)));
}

 

[Ivan Shmakov]

[...]

There's another question, however: what's an easy way to invalidate
the object in an XS DESTROY ()? In Perl, I've seen it being done as
follows:
sub DESTROY {
## .
return
unless ($$self);
## ... clean the things up...
## .
$$self
= undef;
}

I'm not sure exactly what this is intended to accomplish, since
->DESTROY would not be called at all unless $$self was about to be
destroyed. Possibly it's something to do with the out-of-order
destruction that happens during global destruction, but you should
probably be able to clearly articulate the reason this code is
necessary before including it.

Indeed, it's actually done in the ->close () and ->free ()
methods, so to avoid a potential crash should either be called
more than once. (Adding some kind of a "die unless" guard to
all the other XS methods is in the TODO list.)

As of the current revision, ->DESTROY () is aliased to
->close () (i. e., *DESTROY = \&close), and the latter is just
an exception-generating wrapper around the XS ->object_close ():

sub close {
my ($self) = @_;
## .
return
unless ($$self);
my $r
= $self->object_close ();
die (XXX::Exception->new ($r, "object_close"))
unless ($r == 0);
## .
undef;
}

(Or, it could be unless (! defined ($r) || $r == 0), and the
guard avoided, given that ->object_close () results in an early
XSRETURN_EMPTY when passed a reference to an undefined value.)

You don't need to. 'obj' is a SV* pointing to the right SV, unless
you've got a typemap that maps it to something else,

Indeed, I do:

XXX:arent T_PTROBJ
XXX::Child T_OPAQUEPTROBJ

in which case you want to change the 'XXX::Object' to 'SV *'.

This way, I'd need to duplicate the guards now in typemap also
in the .xs code itself, which is what I'd like to avoid.

You only need to access ST() directly if you're trying to change
which SVs are on the stack.

Somehow, I've got an impression that accessing ST () directly is
not something entirely avoided in the XS code. So, unless I
miss some subtle consequence of doing it this way, I'd rather
prefer to keep the XS code uncluttered with some extra guards.

Yes. The equivalent of the Perl above is

SvSetMagicSv(ST(0), &PL_sv_undef);

Indeed, thanks.

(It's rather like sv_setsv_mg (), though, as per sv.c.)

I don't understand what you mean here.

I've meant the "return unless" guard specifically here. Which
now became:

CODE:
if (! SvOK (SvRV (ST (0)))) {
/* . */
XSRETURN_EMPTY;
}

(With typemap also checking SvROK () and sv_derived_from ().)