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

Process::isRunning returns true for defunct tagged process #1097

Open
oblitum opened this issue Dec 22, 2015 · 16 comments
Open

Process::isRunning returns true for defunct tagged process #1097

oblitum opened this issue Dec 22, 2015 · 16 comments
Assignees
Labels
Milestone

Comments

@oblitum
Copy link

oblitum commented Dec 22, 2015

After starting a process with:

Pipe out, err;
auto foo = Process::launch("foo", {}, nullptr, &out, &err);

The following snippet never exits the loop:

while (Process::isRunning(foo)) {
    cout << foo.id() << " still running" << endl;
    sleep_for(1s);
}

I've checked with ps -A | grep foo that the given foo process has been tagged as defunct by the system just after Process::launch, meaning that it has been started and exited successfully, but it still stays around as defunct until the parent application terminates:

.
.
.
32564 pts/15   00:00:00 zsh
32565 pts/5    00:00:00 foo <defunct>
32680 ?        00:10:33 geary
.
.
.

I'm running on x64 Linux with Poco 1.6.1.

@oblitum
Copy link
Author

oblitum commented Dec 22, 2015

I've realized that if foo.wait() gets called before Process::isRunning(foo) then isRunning does return false, meaning that the process stops hanging around as zombie.

I'm not sure what should be the expected behavior. I'm running through all this with Poco because I'm attempting to construct a timed wait(), since Poco doesn't provide one and I can't risk blocking indefinitely.

@oblitum
Copy link
Author

oblitum commented Dec 22, 2015

So my first attempt was to periodically check isRunning and exit on timing out or the child process ending its execution.

@mikedld
Copy link
Member

mikedld commented Jan 7, 2016

That's the reality of this world, you need to call waitpid() for child process to terminate, and Process::isRunning() doesn't do that. I've cooked up a small patch to make Process::isRunning(const ProcessHandle&) call waidpid() but I cannot say I like it much (although it solves the issue).

Ideally we should probably setup a dedicated thread which sits on self-pipe read, calls waitpid() and notifies interested parties (ProcessHandle objects or user-defined listeners) of process status changes. This will be a breaking change though, in that we'll need to register global SIGCHLD handler and thus potentially interfere with other code (external to Poco) which uses either SIGCHLD or waitpid() directly in the same executable.

@mikedld
Copy link
Member

mikedld commented Jan 7, 2016

Note that dedicated thread will also allow for nice Process::tryWait() / ProcessHandle::tryWait() implementation, which is possible on Windows right now but not on *NIX. Another good thing about it would be that users will no longer need to explicitly wait for a process, and this opens the door for detached launch (if you're only interested in launching the process, but not in waiting for it and getting the exit code).

@ServiusHack
Copy link

I just came here for this very same question and don't share @mikedld's view on the reality of this world. A zombie process already has terminated, it consumes almost no memory and will never be on the run queue again. In my understanding such a process is not running anymore. The kernel merely holds a small data structure so somebody can get the exit status and other information.

If the behavior of Process::isRunning() can't be changed a note regarding zombie processes should at least be in the documentation.

Regarding Process::tryWait() on *NIX: There is the option WNOHANG for waitpid which will not block the call if the child has not exited yet.

@oblitum
Copy link
Author

oblitum commented Jan 8, 2016

I'd like to notice that, for example, subprocess.Popen on python provides wait with timeout option.

@mikedld
Copy link
Member

mikedld commented Jan 8, 2016

Committed that small patch I was talking about before. Let's see where it leads us to.

@oblitum
Copy link
Author

oblitum commented Jan 9, 2016

@mikedld Coudn't check it yet but thanks.

@josh-stoddard-tanium
Copy link

It appears that this fix was merged to pocoproject::develop, but has not been merged into any poco release (as of 1.9.1): #1115
Is that intentional?

@aleks-f
Copy link
Member

aleks-f commented Nov 8, 2018

not intentional, please send pull if you want it in the next release

@oblitum
Copy link
Author

oblitum commented Nov 8, 2018

Shouldn't the issue be reopened until there's a plan of merging to releases?

josh-stoddard-tanium pushed a commit to josh-stoddard-tanium/poco that referenced this issue Nov 8, 2018
@josh-stoddard-tanium
Copy link

@aleks-f , PR as requested: #2535

@Icedude907
Copy link

Icedude907 commented Nov 28, 2024

Never fixed. This is truly the bad ending. RIP Josh's PR.

To others looking for a workable solution - you could wait for a Poco::Pipe attached to the process' stdout to return 0, or check for Process::try_wait(hndl) != -1.

@matejk
Copy link
Contributor

matejk commented Nov 28, 2024

@Icedude907, would you prepare a PR that would solve the problem and help others that are affected?

@Icedude907
Copy link

Hey @matejk, thanks for reaching out. I have found a solution I am happy with so I don't feel a pressing need to try fixing the issue upstream.
Thank you for all the work you do for this project.
Have a good day!

@aleks-f aleks-f changed the title Poco::Process::isRunning always returning true while process has been tagged defunct Process::isRunning returns true for defunct tagged process Dec 7, 2024
@aleks-f
Copy link
Member

aleks-f commented Dec 7, 2024

@Icedude907 at the time, Josh was asked for some further changes, but he had no time. Now, 6 years later, you complain about it but also can't be bothered to contribute. FWIW, here is the change adapted to the most recent release; maybe someone will find time to put it in order. I only checked on mac and tests do not pass.

@aleks-f aleks-f reopened this Dec 7, 2024
@aleks-f aleks-f added this to the Unspecified milestone Dec 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants