Unity System Compositor

Merge lp://qastaging/~afrantzis/unity-system-compositor/no-external-spinner-zombie-processes into lp://qastaging/unity-system-compositor

  1. no-external-spinner-zombie-processes
  2. Merge into trunk
Proposed by Alexandros Frantzis
Status: Work in progress
Proposed branch: lp://qastaging/~afrantzis/unity-system-compositor/no-external-spinner-zombie-processes
Merge into: lp://qastaging/unity-system-compositor
Diff against target: 271 lines (+180/-15)
5 files modified
src/external_spinner.cpp (+85/-4)
tests/integration-tests/CMakeLists.txt (+1/-0)
tests/integration-tests/spin_wait.cpp (+39/-0)
tests/integration-tests/spin_wait.h (+39/-0)
tests/integration-tests/test_external_spinner.cpp (+16/-11)
To merge this branch: bzr merge lp://qastaging/~afrantzis/unity-system-compositor/no-external-spinner-zombie-processes
Reviewer Review Type Date Requested Status
Unity System Compositor Development Team Pending
Review via email: mp+264281@code.qastaging.launchpad.net

Commit message

Don't leave zombie spinner processes

Description of the change

Don't leave zombie spinner processes

This MP uses the double-fork trick to avoid zombie spinner process. It works by forking an intermediate process which then forks again to execute the real spinner process. The intermediate process exits immediately, orphaning the spinner process, which is then adopted by init (pid 1). The init process then takes care of waiting for the process to exit.

To post a comment you must log in.
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Haven't looked at the branch in detail, but I'm surprised it needs much work at all... Simply calling wait() or waitpid() at the right times will avoid zombies. That's what "zombie process" means -- that the parent has not called wait() yet.

Reply
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

> Simply calling wait() or waitpid() at the right times will avoid zombies.
> That's what "zombie process" means -- that the parent has not called wait() yet.

That was my initial approach, but the problem with this is that waitpid() blocks USC for a little bit in the best case, and can block forever if for whatever reason the external spinner happens to not respond to our signals (e.g. it has hung).

The proposed way is indeed more complicated, but it avoid all blocking problems by transferring the responsibility to the init process.

Note that a lot of the code in this MP is just test improvements, and signal-safe wrappers around read/write.

Reply
Revision history for this message
Chris Halse Rogers (raof) wrote :

On Friday, 10 July 2015 6:21:03 PM AEST, Alexandros Frantzis wrote:
>> Simply calling wait() or waitpid() at the right times will avoid zombies.
>> That's what "zombie process" means -- that the parent has not
>> called wait() yet.
>
> That was my initial approach, but the problem with this is that
> waitpid() blocks USC for a little bit in the best case, and can
> block forever if for whatever reason the external spinner
> happens to not respond to our signals (e.g. it has hung).

The canonical way of handling this is a SIGCHILD handler; that way you only
ever call non-blocking waitpid.

That said, double-forking should work, too.

--
Sent using Dekko from my Ubuntu device

Reply
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

> The canonical way of handling this is a SIGCHILD handler; that way you only
> ever call non-blocking waitpid.

I considered this, but was concerned about interaction with libmirserver signal handling. If we think it is safe, though, I would rather use SIGCHILD too. I will propose an alternative MP with sigchild and we can decide.

Reply
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Alternative MP using sigchld:

https://code.launchpad.net/~afrantzis/unity-system-compositor/no-external-spinner-zombie-processes-sigchld/+merge/264410

Also, I split out the test fixes:

https://code.launchpad.net/~afrantzis/unity-system-compositor/fix-1473418-external-spinner-tests/+merge/264411

Reply

Unmerged revisions

224. By Alexandros Frantzis

Don't leave zombie spinner processes

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches