Feature 'class' Segmentation fault in DESTROY phase for improperly constructed instance about perl5 HOT 6 CLOSED

I do get the Required parameter 'dependency' is missing for "Foo" constructor error, but then DESTROY is called anyway and segfaults. I think the issue is that the destructor is run at all for objects whose constructor hasn't finished successfully.

from perl5.

Good spot, I've amended the report.

@ilmari also pointed out, which might be useful for those looking for this type of lifecycle management, that spelling it method DESTROY will allow access to class members directly.

from perl5.

Building a debug perl yields this:

$ ./perl -Ilib gh22278.pl 
Required parameter 'dependency' is missing for "Foo" constructor at gh22278.pl line 14.
perl: class.c:299: Perl_pp_methstart: Assertion `fieldp[fieldix]' failed.
Aborted (core dumped)

Indeed the issue is that the object isn't fully-constructed at this point, so some of its SV pointers in the fields array are still NULL.

There's two potential approaches to fixing this:

1. pp_methstart can be more defensive and skip over fields whose pointers are still NULL. This would incur a (very) slight performance cost at every method entry point, having to check all those fields, though I suspect it's small measurement noise at that point, or
2. Instance construction could be split into two phases. The first phase would populate every scalar/array/hash field with a fresh but empty container variable, before actually running any of the initialisation code. That would involve walking the entire ancestry twice during construction.

Overall I feel that option 1 is probably better here, as it involves far smaller code change. A single extra NULL pointer comparison is unlikely to make a huge impact as compared to making all constructors quite a bit slower.

from perl5.

But why is the destructor called at all?

from perl5.

But why is the destructor called at all?

Perl calls sub DESTROY when it throws away an object instance. There's not a lot we can do about that here.

Certainly we could create (and encourage folks to use) a separate DESTRUCT phaser that is a phaser, not a method, and ensure each one only gets invoked after that particular part of the constructor has completed successfully. In most cases that would be what you wanted to use. But aside from far deeper changes elsewhere, we can't do much about DESTROY so we have to be prepared to handle it.

from perl5.

It seems the wider topic of how destructors ought to work is still a thing that needs discussing, though I don't think this bug is the place to do it. I'd suggest we begin a separate discussion about that to look at the wider topic overall.

from perl5.