perl5 icon indicating copy to clipboard operation
perl5 copied to clipboard

Published 20 hours ago • verified publisher icon Perl

Add PL_shutdownhook

Open Leont opened this issue 4 years ago • 16 comments

This is a proposed solution to #19022

This adds a hook that is run early in PERL_SYS_TERM to terminate global resources, and uses it to destroy the shared interpreter in threads::shared

Leont avatar Aug 06 '21 14:08 Leont

On Fri, Aug 06, 2021 at 07:57:06AM -0700, Leon Timmermans wrote:

This is a proposed solution to #19022

This adds a hook that is run early in PERL_SYS_TERM to terminate global resources, and uses it to destroy the shared interpreter in threads::shared You can view, comment on, or merge this pull request online at:

Any particular reason PL_shutdownhook is manually declared in perl.h rather than being in perlvars.h like other global vars?

-- Never do today what you can put off till tomorrow.

iabyn avatar Aug 07 '21 10:08 iabyn

Any particular reason PL_shutdownhook is manually declared in perl.h rather than being in perlvars.h like other global vars?

I didn't know about that, but that does seem like a better idea. I copied it from something else that also wasn't in perlvars.h (but I forgot what)

Leont avatar Aug 07 '21 14:08 Leont

Any particular reason PL_shutdownhook is manually declared in perl.h rather than being in perlvars.h like other global vars?

Fixed that.

Leont avatar Aug 13 '21 11:08 Leont

Aside from my comment about reflowing a macro, this LGTM

leonerd avatar Aug 22 '21 13:08 leonerd

could you please document PL_shutdownhook?

tonycoz avatar Sep 08 '21 04:09 tonycoz

So, are we just waiting for @Leont to document this before it is ready to merge?

khwilliamson avatar Dec 16 '21 15:12 khwilliamson 

* leont is not sure where/how to document PL_shutdownhook, given that 
          none of the other similar hooks are documented.

I'm not so worried about where, more about adding another undocumented public hook.

Things to document:

  • when is the hook called? Do I need to chain to the old hook?
  • what do I need to lock to set it? (though a simple API to set (or add/remove a given function pointer) it might be better, since hooks have led to some bugs in the past)

tonycoz avatar Dec 20 '21 02:12 tonycoz

@leont, this looks close to being ready

khwilliamson avatar Mar 04 '22 18:03 khwilliamson

@Leont, this looks close to being ready

@LeonT, can we get an update on the status of this pull request?

jkeenan avatar Jul 03 '22 18:07 jkeenan

@Leont, can we get an update on the status of this pull request?

I'm adapting it to tony's suggestion (simple API to set it), will update it soon

Leont avatar Jul 03 '22 19:07 Leont

@leont last message in this PR is that you were adapting it to feedback. Do you still want to keep this PR? (It seems reasonable to me in principal).

demerphq avatar Feb 08 '23 07:02 demerphq

Do you still want to keep this PR? (It seems reasonable to me in principal).

Yeah, it's a problem that needs a solution. I'll try to pick it up again soon.

Leont avatar Feb 10 '23 21:02 Leont

@Leont I'm happy to get it rebased for you considering you seem busy of late.

demerphq avatar Feb 11 '23 02:02 demerphq

I'm happy to get it rebased for you considering you seem busy of late.

It needs much more than a rebase, otherwise this would have been merged already. I'll try to finish this soon.

Leont avatar Feb 25 '23 14:02 Leont