Tcl-bounties icon indicating copy to clipboard operation
Tcl-bounties copied to clipboard

TIP #462 completed and merged into the core (was: Intend to work on "Stop Tcl from eating child process exit status gratuitously")

Open fredericbonnet opened this issue 8 years ago • 20 comments

fredericbonnet avatar Jan 06 '17 12:01 fredericbonnet

Howdy,

I'd like some clarifications about the needed features. I've had a quick look at the fa_sudo package for insight, and especially the exec_as -returnall proc option. Would a native implementation of a similar option cover the original intent (retrieving child process status asynchronously) or do you need something more elaborate, such as introspection functions about child processes through e.g. a new info subcommand?

Fred

fredericbonnet avatar Jan 06 '17 13:01 fredericbonnet

You would need some kind of mechanism to report (through asynchronous callbacks) or query (though introspection) the creation of children. A reporting mechanism would enable the creation of an efficient introspection mechanism in Tcl (maintain an array or list of reported procs, update it as they return), but it would be preferable to have this a native part of the system.

resuna avatar May 11 '17 14:05 resuna

Note that most of fa_sudo only exists because currently, tcl's tendency to eat exit status means that if you care about asynchronous exit status, you have to reimplement it all yourself and entirely avoid use of the core's subprocess tools (piped-open, exec)

If the standard facilities stopped doing that or provided a mechanism to retrieve statuses that had been consumed by the core (i.e. a replacement for waitpid) then most of fa_sudo could go away and you'd just have a thin wrapper around piped-open

mutability avatar May 11 '17 14:05 mutability

I've submitted TIP #462 to address this issue:

http://www.tcl.tk/cgi-bin/tct/tip/462

I plan to start working on an implementation in the next few days.

Please note: the TIP title is outdated but there is no way to change it once a TIP has been submitted. Title should be "Add New ::tcl::process Ensemble for Subprocess Management"

fredericbonnet avatar May 11 '17 15:05 fredericbonnet

Nice.

resuna avatar May 11 '17 15:05 resuna

Hi, just to let you know that I have a preliminary TIP #462 implementation that works fine on all detached processed spawn by [exec &] and [open |]. I have a local fossil branch based on the Tcl 8.7 core but I'd like to share it with you for evaluation before submitting to TCL-CORE.

An important precision: the implementation only works on DETACHED processes. This implies that the pipe channel be set to non-blocking mode then closed. Blocking-mode pipes can still use the standard synchronous Tcl error handling mechanism. For this reason I don't think it would make sense for the TIP to support non-detached processes anyway, but I welcome any sensible argument.

Fred

fbonnet-creative avatar Aug 23 '17 20:08 fbonnet-creative

Can you use the advanced monitoring stuff on a non-blocking, non-closed pipeline? EOF of a pipeline and process exit are distinct events so when working with a pipeline in nonblocking mode you need a mechanism to handle both asynchronously

mutability avatar Aug 23 '17 20:08 mutability

Also, when you say it only worked on DETACHED processes: how do other child PIDs behave, do they show up with no data or not appear at all or what?

mutability avatar Aug 23 '17 20:08 mutability

For now it only works on non-blocking, closed pipelines (that's the only way to detach the procs); non-detached PIDs don't appear in the list. So the implementation excludes both use cases above, as non-detached procs use a different code path in Tcl. However it should be possible to make it work with extra effort, I've tried my best to limit the impact on existing code and put all new code in a specific file tclProcess.c.

fbonnet-creative avatar Aug 23 '17 21:08 fbonnet-creative

Just realized that I've used the wrong GitHub account, oh well.

fredericbonnet avatar Aug 24 '17 05:08 fredericbonnet

Hi again,

I've refactored the code so that it can handle all kinds of processes now. Again, the impact on Tcl core is limited. I've done some basic testing on Windows and everything seems to work OK.

I've made a small change to the [tcl::process status] documented in the TIP: it now returns a {code msg errorCode} triplet for completed processes instead of just the errorCode.

Please let me know in which format I can send you the changes for evaluation. There are 4 changed core files and a new C source for the TIP (plus a preliminary test file).

Fred

fredericbonnet avatar Aug 28 '17 07:08 fredericbonnet

I've just remembered that you are located in Houston, I hope everybody is well and safe on your side.

fredericbonnet avatar Aug 28 '17 16:08 fredericbonnet

I hope everybody is well and safe

I've got recently a message from Peter (@resuna):

Weathered so far so good... Everyone at Flightaware has now checked in safe

sebres avatar Aug 28 '17 17:08 sebres

Thank you. I am back in the office this morning and will look into this once I get a chance.

resuna avatar Aug 31 '17 15:08 resuna

Hey,

just to let you know that I've pushed my code to the Tcl Fossil repo, on branch 'tip-462'. I've also updated the TIP accordingly.

fredericbonnet avatar Nov 05 '17 21:11 fredericbonnet

Hi all, happy new year 2018 ! Have you had the time to experiment with the feature? I'd like to get real-world feedback before calling for vote on TIP 462.

fredericbonnet avatar Jan 01 '18 14:01 fredericbonnet

It looks solid to me, I've asked Karl to have a look too.

resuna avatar Jan 16 '18 15:01 resuna

They're discussing it on the list. I say strike now.

resuna avatar Jan 25 '18 02:01 resuna

Hi !

I'm glad to announce that I've completed the implementation of TIP #462 (accepted on 2018-03-12) and that the changes have been merged into the branch core-8-branch. The code has already been there for some time, but the man page and the test suite were still missing. So now the tcl::process command is an integral part of Tcl 8.7!

Is there anything else that needs to be done to fulfill the conditions for this bounty?

fredericbonnet avatar May 10 '18 21:05 fredericbonnet

Contact me in email.

resuna avatar May 11 '18 12:05 resuna