[perl #18219] on_exit not portable
Andy Dougherty
# Please include the string: [perl #18219]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org/rt2/Ticket/Display.html?id=18219 >
The Solaris tinderbox is currently failing with
gcc -o parrot [...]
Undefined first referenced
symbol in file
on_exit interpreter.o
That's because Solaris doesn't have on_exit. It does have atexit, but
atexit doesn't take any parameters so it's not a direct drop-in
replacement. The correct "fix" is not obvious to me. I suppose
someone could have Configure.pl test for on_exit and only use it if
available, but it'd probably be better to rework interpreter.c to use
atexit() if possible.
--
Andy Dougherty doug...@lafayette.edu
Leopold Toetsch
> That's because Solaris doesn't have on_exit. It does have atexit, but
> atexit doesn't take any parameters so it's not a direct drop-in
> replacement. The correct "fix" is not obvious to me. I suppose
> someone could have Configure.pl test for on_exit and only use it if
> available, but it'd probably be better to rework interpreter.c to use
> atexit() if possible.
The on_exit/atexit is currently necessary to clean up behind exceptions.
If you don't mind memory leaks after exceptions, remove the
#define ATEXIT_DESTROY
at the beginning of interpreter.c, until we have a config test for on_exit.
atexit is not an alternative, because we might have multiple
interpreters to clean up like in t/op/interp_2.
leo
Josh Wilmes
> atexit is not an alternative, because we might have multiple
> interpreters to clean up like in t/op/interp_2.
So the issue here is that on_exit can take a parameter to be passed into
the handler function, right?
We could implement our own version of on_exit that registered the handlers/
arguments in a linked list or something, and have a single atexit() handler
call them on our behalf.
However, that still assumes we have atexit() everywhere. This appears to
not be true on SunOS at least- apparently it has on_exit, though.
But, it seems like we can make this work everywhere if we move to using a
platform.c Parrot_on_exit() and Parrot_exit() rather than any kind of
native on_exit/atexit/exit functions.
Then we can make them all DTRT everywhere, I think... either with our
without atexit().
If this (Parrot_exit/Parrot_on_exit) is a reasonable way to do things, I
can probably come up with a patch later this week, if nobody else jumps on
it ;-)
--Josh
Leopold Toetsch
> At 18:57 on 11/04/2002 +0100, Leopold Toetsch <l...@toetsch.at> wrote:
>>atexit is not an alternative, because we might have multiple
>>interpreters to clean up like in t/op/interp_2.
> So the issue here is that on_exit can take a parameter to be passed into
> the handler function, right?
Yep, we need the interpreter as argument.
> We could implement our own version of on_exit that registered the handlers/
> arguments in a linked list or something, and have a single atexit() handler
> call them on our behalf.
We could put this list into the interpreter structure itself, so that
created interpreters would be chained together. Actually, the
<runinterp> ops used in t/op/interp_2 runs the code of the parent
interpreter for execution, so there might always be some kind of reusal
of structures, which chained interpreters have to know in _destroy.
Same is with my experiment of a separate interpreter for the tracing
subsystem (currently disabled, s. runops_cores.c).
> If this (Parrot_exit/Parrot_on_exit) is a reasonable way to do things, I
> can probably come up with a patch later this week, if nobody else jumps on
> it ;-)
Would be greate, thanks.
> --Josh
leo
Nicholas Clark
> At 18:57 on 11/04/2002 +0100, Leopold Toetsch <l...@toetsch.at> wrote:
>
> > atexit is not an alternative, because we might have multiple
> > interpreters to clean up like in t/op/interp_2.
>
> So the issue here is that on_exit can take a parameter to be passed into
> the handler function, right?
>
> We could implement our own version of on_exit that registered the handlers/
> arguments in a linked list or something, and have a single atexit() handler
> call them on our behalf.
>
> However, that still assumes we have atexit() everywhere. This appears to
> not be true on SunOS at least- apparently it has on_exit, though.
IIRC ANSI C89 says that the library provides atexit()
If SunOS doesn't want to be C89 compliant, then I have no qualms about
telling the first SunOS porter that we'd be pleased to accept patches from
*them* to work around it. (And the return value from sprintf in SunOS, if
we need that.)
> But, it seems like we can make this work everywhere if we move to using a
> platform.c Parrot_on_exit() and Parrot_exit() rather than any kind of
> native on_exit/atexit/exit functions.
That seems to be a better plan.
> Then we can make them all DTRT everywhere, I think... either with our
> without atexit().
>
> If this (Parrot_exit/Parrot_on_exit) is a reasonable way to do things, I
> can probably come up with a patch later this week, if nobody else jumps on
> it ;-)
I'm not jumping. Sorry.
Nicholas Clark
PS Yes, I may sound beligerant and unhelpful. But implementors have had
>12 years to get C89 correct. C99 is more complex - I wonder how long
long that will take...
--
Slide rules better than perl? http://www.perl.org/advocacy/spoofathon/
Josh Wilmes
> On Mon, Nov 04, 2002 at 07:45:46PM -0500, Josh Wilmes wrote:
> > However, that still assumes we have atexit() everywhere. This appears to
> > not be true on SunOS at least- apparently it has on_exit, though.
>
> IIRC ANSI C89 says that the library provides atexit()
> If SunOS doesn't want to be C89 compliant, then I have no qualms about
> telling the first SunOS porter that we'd be pleased to accept patches from
> *them* to work around it. (And the return value from sprintf in SunOS, if
> we need that.)
I agree. However, the point is fairly moot.. If we're going to do a
Parrot_on_exit, it's just as easy to provide our own Parrot_exit and not
need atexit() either.. it's not like atexit() is giving us much at that
point.
--Josh
Leopold Toetsch
> I agree. However, the point is fairly moot.. If we're going to do a
> Parrot_on_exit, it's just as easy to provide our own Parrot_exit and not
> need atexit() either.. it's not like atexit() is giving us much at that
> point.
.... which would mean, that internal_exception needs an Parrot_interp*
argument - which it will need anyway to do something useful finally.
> --Josh
leo
Josh Wilmes
Not necessarily... I was thinking that Parrot_exit/Parrot_on_exit would
have the same signatures as their libc equivalents. There should not be
a need to introduce an interpreter in Parrot_exit().. It wouldn't hurt,
but I don't think it's particularly necessary, if each interpreter has
registered an on_exit handler..
--Josh
Josh Wilmes
> Well, I got bit this week by the on_exit stuff. I'm still not sure
> why we need this. Could someone please explain, so I don't have to
> yank it out?
Leo said:
> The on_exit/atexit is currently necessary to clean up behind exceptions.
> If you don't mind memory leaks after exceptions, remove the
>
> #define ATEXIT_DESTROY
>
> at the beginning of interpreter.c, until we have a config test for on_exit.
>
> atexit is not an alternative, because we might have multiple
> interpreters to clean up like in t/op/interp_2.
How about we just remove that define for now for now, and let it leak that
little bit for now?
Presumably leo can add the ATEXIT_DESTROY to his build if he's trying to
track down leaks.
I'll try to find some time to write a more portable Parrot_on_exit/
Parrot_exit implementation later this week.
Then we can get this cleanup behavior back on by default. At the moment,
the tinderbox is a mess, which isn't cool.
--Josh
Dan Sugalski
Well, I got bit this week by the on_exit stuff. I'm still not sure
why we need this. Could someone please explain, so I don't have to
yank it out?
--
Dan
--------------------------------------"it's like this"-------------------
Dan Sugalski even samurai
d...@sidhe.org have teddy bears and even
teddy bears get drunk
Josh Wilmes
> >Leo said:
> >
> >> The on_exit/atexit is currently necessary to clean up behind exceptions.
> >> If you don't mind memory leaks after exceptions, remove the
>
> Right, I saw that, I just don't understand why. If it's in as a fix
> for the current busted state of exceptions, then we need to fix
> exceptions, I think.
I was operating on the assumption that some sort of global cleanup was a
necessity for some reason. If it's a workaround for a bug in exceptions,
then I agree.
> For now, I think I'd rather leak, as it is definitely killing the tinderbox.
Agreed.
--Josh
Dan Sugalski
>At 13:41 on 11/06/2002 EST, Dan Sugalski <d...@sidhe.org> wrote:
>
>> Well, I got bit this week by the on_exit stuff. I'm still not sure
>> why we need this. Could someone please explain, so I don't have to
>> yank it out?
>
>Leo said:
>
>> The on_exit/atexit is currently necessary to clean up behind exceptions.
>> If you don't mind memory leaks after exceptions, remove the
>>
>> #define ATEXIT_DESTROY
>>
>> at the beginning of interpreter.c, until we have a config test for on_exit.
>>
>> atexit is not an alternative, because we might have multiple
>> interpreters to clean up like in t/op/interp_2.
Right, I saw that, I just don't understand why. If it's in as a fix
for the current busted state of exceptions, then we need to fix
exceptions, I think.
For now, I think I'd rather leak, as it is definitely killing the tinderbox.
--
Leopold Toetsch
> At 1:47 PM -0500 11/6/02, Josh Wilmes wrote:
>
>> At 13:41 on 11/06/2002 EST, Dan Sugalski <d...@sidhe.org> wrote:
>>
>>> Well, I got bit this week by the on_exit stuff. I'm still not sure
>>> why we need this. Could someone please explain, so I don't have to
>>> yank it out?
>>
>>
>> Leo said:
>>
>>> The on_exit/atexit is currently necessary to clean up behind
>>> exceptions.
>>> If you don't mind memory leaks after exceptions, remove the
>>>
>>> #define ATEXIT_DESTROY
>>>
>>> at the beginning of interpreter.c, until we have a config test for
>>> on_exit.
>>>
>>> atexit is not an alternative, because we might have multiple
>>> interpreters to clean up like in t/op/interp_2.
>>
>
> Right, I saw that, I just don't understand why. If it's in as a fix for
> the current busted state of exceptions, then we need to fix exceptions,
> I think.
Summary of changes I did put in WRT this:
- interpreter startup code is reordered so that all PMCs/Buffers are in
managed memory now.
- Long running programs using e.g. new interpreters on the fly would
currently leak tons of memory.
- Debugging memory corruptions is a pain, when there is a lot of leaking
memory
- freeing all used memory is a good measure against corrupted mem, so
- I implemented Parrot_destroy, which cleans up all internal data
structures of an interpreter and frees all used mem.
This is currently ok for:
Failed Test Status Wstat Total Fail Failed List of failed
-------------------------------------------------------------------------------
t/op/hacks.t 2 1 50,00% 1
t/op/rx.t 23 7 30,43% 8-9, 12-14, 21-22
t/pmc/pmc.t 82 4 4,88% 60-62, 74
9 subtests skipped.
Failed 3/34 test scripts, 91.18% okay. 12/494 subtests failed, 97.57% okay.
(Above is produced by "testyamd", which includes mem leaks as failures)
Above leaks are known why/where they come from.
Not too few tests use exceptions to show some wanted error message, so I
used atexit first, to catch these.
on_exit() jumped in, with the problem: how to clean up multiple
interpreters, where a simple atexit() can't provide the necessary
interpreter argument. 2 possible solutions are: a chained interpreter
list structure keeping track of running interpreters, or an extra
interpreter argument in internal_exceptions.
> For now, I think I'd rather leak, as it is definitely killing the
> tinderbox.
This is was I did say above, just put comments around above statement if
tinderboxen are the concern.
leo
Dan Sugalski
>Summary of changes I did put in WRT this:
>- interpreter startup code is reordered so that all PMCs/Buffers are
>in managed memory now.
>- Long running programs using e.g. new interpreters on the fly would
>currently leak tons of memory.
>- Debugging memory corruptions is a pain, when there is a lot of
>leaking memory
>- freeing all used memory is a good measure against corrupted mem, so
>- I implemented Parrot_destroy, which cleans up all internal data
>structures of an interpreter and frees all used mem.
Right, I understand. (And I really appreciate it--this was a hanging
area that didn't get nearly the proper attention it needed) The bit I
don't understand is the need for a hook into the process exit.
Generally having explicit interpreter cleanup is OK, though if you've
got a reason to not trust that I can be convinced otherwise.
>>For now, I think I'd rather leak, as it is definitely killing the tinderbox.
>
>
>This is was I did say above, just put comments around above
>statement if tinderboxen are the concern.
Yeah, I think I'll do that for right now. What I'd like is a probe
for this in configure. Oh, Brent.... :)
Leopold Toetsch
>> - I implemented Parrot_destroy, which cleans up all internal data
>> structures of an interpreter and frees all used mem.
> Right, I understand. (And I really appreciate it--this was a hanging
> area that didn't get nearly the proper attention it needed) The bit I
> don't understand is the need for a hook into the process exit.
Then you should have read on ;-)
~8% tests use internal_exception to deliver a test result. Not catching
the exception may hide memory leaks in such tests.
More detailled:
Testing for memory leaks uses the same tests as are already in t/*. A
test is passed, when the result is ok, and when it doesn't leak mem.
This is accomplished by the scipt "testyamd" I checked in. When the
exceptions just exits and Parrot_destroy isn't reached, the test would fail.
> Yeah, I think I'll do that for right now. What I'd like is a probe for
> this in configure. Oh, Brent.... :)
I did deactivate it some minutes ago. Tinderboxen should be happy again.
Waiting for the config test - but it's not an urgent one.
leo
Josh Wilmes
> >This is was I did say above, just put comments around above
> >statement if tinderboxen are the concern.
>
> Yeah, I think I'll do that for right now. What I'd like is a probe
> for this in configure. Oh, Brent.... :)
If an on-exit cleanup is required for parrot to not leak memory, then
simply probing for on_exit in configure won't be sufficient; that's why a
platform.c implementation of Parrot_[on_]exit would be needed.
--Josh
Josh Wilmes
to CVS.
This will fix the leak on all platforms, for now. If you want to fix
internal_exception so this isn't necessary, that's fine- we can rip this
out later.
--Josh
Leopold Toetsch
> For the meantime, I have added the Parrot_exit and Parrot_on_exit functions
> to CVS.
Thanks for providing this. I did slightly modify your patch to really
get rid of the leaks:
- test_main calls Parrot_exit now
- copied prototype to embed.h
leo
Josh Wilmes
> Josh Wilmes wrote:
>
> > For the meantime, I have added the Parrot_exit and Parrot_on_exit function
s
> > to CVS.
>
> Thanks for providing this. I did slightly modify your patch to really
> get rid of the leaks:
> - test_main calls Parrot_exit now
Huh- i have no idea how i missed that one.. thanks!
> - copied prototype to embed.h
Righto.
--Josh