Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
fredericbonnet opened this issue Jan 6, 2017 · 20 comments
Assignees

Comments

@fredericbonnet
Copy link

No description provided.

@fredericbonnet
Copy link
Author

fredericbonnet commented Jan 6, 2017

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

@resuna
Copy link
Member

resuna commented May 11, 2017

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.

@mutability
Copy link

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

@fredericbonnet
Copy link
Author

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"

@resuna
Copy link
Member

resuna commented May 11, 2017

Nice.

@fbonnet-creative
Copy link

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

@mutability
Copy link

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
Copy link

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?

@fbonnet-creative
Copy link

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.

@fredericbonnet
Copy link
Author

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

@fredericbonnet
Copy link
Author

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
Copy link
Author

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

@sebres
Copy link

sebres commented Aug 28, 2017

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

@resuna
Copy link
Member

resuna commented Aug 31, 2017

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

@fredericbonnet
Copy link
Author

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
Copy link
Author

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.

@resuna
Copy link
Member

resuna commented Jan 16, 2018

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

@resuna
Copy link
Member

resuna commented Jan 25, 2018

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

@fredericbonnet fredericbonnet changed the title Intend to work on "Stop Tcl from eating child process exit status gratuitously" TIP #462 completed and merged into the core (was: Intend to work on "Stop Tcl from eating child process exit status gratuitously") May 10, 2018
@fredericbonnet
Copy link
Author

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?

@resuna
Copy link
Member

resuna commented May 11, 2018

Contact me in email.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants