process-cpp

Merge lp:~thomas-voss/process-cpp/fix-death-observer into lp:process-cpp

  1. fix-death-observer
  2. Merge into trunk
Proposed by Thomas Voß
Status: Merged
Approved by: Łukasz Zemczak
Approved revision: 54
Merged at revision: 45
Proposed branch: lp:~thomas-voss/process-cpp/fix-death-observer
Merge into: lp:process-cpp
Diff against target: 1084 lines (+538/-262)
12 files modified
CMakeLists.txt (+2/-2)
debian/changelog (+9/-0)
debian/control (+2/-2)
debian/libprocess-cpp1.symbols (+58/-50)
include/core/posix/child_process.h (+8/-12)
include/core/posix/signal.h (+61/-0)
src/CMakeLists.txt (+1/-0)
src/core/posix/child_process.cpp (+90/-160)
src/core/posix/signal.cpp (+217/-0)
tests/CMakeLists.txt (+15/-0)
tests/death_observer_test.cpp (+36/-0)
tests/posix_process_test.cpp (+39/-36)
To merge this branch: bzr merge lp:~thomas-voss/process-cpp/fix-death-observer
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Colin Watson Approve
Marcus Tomlinson (community) Approve
Michal Hruby (community) Approve
Łukasz Zemczak Pending
Review via email: mp+211996@code.launchpad.net

Commit message

Fix DeathObserver implementation to work correctly in multi-threaded cases.

Description of the change

Fix DeathObserver implementation to work correctly in multi-threaded cases.

 * Are there any related MPs required for this MP to build/function as expected? No
 * Did you perform an exploratory manual test run of your code change and any related functionality? Yes
 * If you changed the packaging (debian), did you subscribe a core dev to the changes? Yes
 * Did you rebuild all reverse dependencies of process-cpp and checked that all of them build correctly? Yes, all of them building correctly.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:43
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/46/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/46
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/46
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/46/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/42

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/46/rebuild

review: Approve (continuous-integration)
Revision history for this message
Michal Hruby (mhr3) wrote :

LGTM

review: Approve
Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

84 + fds[0] = {scope.signal_fd, POLLIN, 0};
85 fds[1] = {wakeup_fd, POLLIN, 0};
86
87 while (true)
88 {
89 + fds[0] = {scope.signal_fd, POLLIN, 0};
90 + fds[1] = {wakeup_fd, POLLIN, 0};

Assigning the fds outside of the loop is redundant.

129 + switch(signal_info[i].ssi_signo)

This is dangerous when more than 5 signals are received. We should have limit checking.

Other than those, this looks good man.

review: Needs Fixing
Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

Sorry, ignore my previous comment about:

129 + switch(signal_info[i].ssi_signo)

I misunderstood the sizeof call in ::read

lp:~thomas-voss/process-cpp/fix-death-observer updated
44. By Thomas Voß

Factor out os signal handling into class SignalTrap.
Refactor ChildProcess::DeathObserver and remove polling on signalfd.
Adjust test cases.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:44
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/47/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/47
    FAILURE: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/47/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/43/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/47/rebuild

review: Needs Fixing (continuous-integration)
lp:~thomas-voss/process-cpp/fix-death-observer updated
45. By Thomas Voß

Make sure that a death observer is always setup correctly.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:45
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/48/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/48
    FAILURE: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/48/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/44/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/48/rebuild

review: Needs Fixing (continuous-integration)
lp:~thomas-voss/process-cpp/fix-death-observer updated
46. By Thomas Voß

Adjust platform-specific symbols.

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

One observation so far:

518 + if (has_been_created_once.exchange(true))
519 + throw std::runtime_error
520 + {
521 + "DeathObserver::create_once_with_signal_trap: "
522 + "Cannot create more than one instance."
523 + };
524 +
525 + std::unique_ptr<core::posix::ChildProcess::DeathObserver> result
526 + {
527 + new DeathObserverImpl{trap}
528 + };

If line 527 throws (hence the death observer is not created) we can't ever create one as has_been_created_once is already set true (518).

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:46
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/49/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/49
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/49
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/49/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/45

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/49/rebuild

review: Approve (continuous-integration)
lp:~thomas-voss/process-cpp/fix-death-observer updated
47. By Thomas Voß

Make sure that we can retry death observer creation in case of c'tor throwing.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:47
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/50/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/50
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/50
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/50/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/46

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/50/rebuild

review: Approve (continuous-integration)
lp:~thomas-voss/process-cpp/fix-death-observer updated
48. By Thomas Voß

Add tests for ChildProcess::DeathObserver.
Remove obsolete coud.

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

Are you getting "warning: no matching file member found for..." for the following methods in 'signal.cpp':

768 +std::shared_ptr<core::posix::SignalTrap> core::posix::trap_signals_for_process(
769 + std::initializer_list<core::posix::Signal> blocked_signals)
...
776 +std::shared_ptr<core::posix::SignalTrap> core::posix::trap_signals_for_all_subsequent_threads(
777 + std::initializer_list<core::posix::Signal> blocked_signals)

If so, to fix this, the declarations need a full specification of Signal for the initializer_list parameters in 'signal.h':

250 +std::shared_ptr<SignalTrap> trap_signals_for_process(
251 + std::initializer_list<Signal> blocked_signals);

should be:

250 +std::shared_ptr<SignalTrap> trap_signals_for_process(
251 + std::initializer_list<core::posix::Signal> blocked_signals);

and...

258 +std::shared_ptr<SignalTrap> trap_signals_for_all_subsequent_threads(
259 + std::initializer_list<Signal> blocked_signals);

should be:

258 +std::shared_ptr<SignalTrap> trap_signals_for_all_subsequent_threads(
259 + std::initializer_list<core::posix::Signal> blocked_signals);

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:48
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/51/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/51
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/51
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/51/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/47

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/51/rebuild

review: Approve (continuous-integration)
lp:~thomas-voss/process-cpp/fix-death-observer updated
49. By Thomas Voß

Scope Signal as core::posix::Signal.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:49
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/52/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/52
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/52
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/52/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/48

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/52/rebuild

review: Approve (continuous-integration)
lp:~thomas-voss/process-cpp/fix-death-observer updated
50. By Thomas Voß

Do not set but block mask.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:50
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/53/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/53/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/53
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/53/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/49

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/53/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

Nice catch! This is looking good now :) Perhaps just a re-trigger on Jenkins and we're done here.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:50
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/54/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/54
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/54
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/54/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/50

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/54/rebuild

review: Approve (continuous-integration)
lp:~thomas-voss/process-cpp/fix-death-observer updated
51. By Thomas Voß

* Bump so name.
* Fix death observer racyness.
* Factor out signalfd-based wait operations on operating system signals.
* Introduce interface SignalTrap and accompanying factory functions.

52. By Thomas Voß

[ thomas-voss ]
Only generate docs in release mode by default.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:52
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/55/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/55
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/55
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/55/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/51

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/55/rebuild

review: Approve (continuous-integration)
Revision history for this message
Colin Watson (cjwatson) wrote :

Nothing seems to close the event_fd at any point.

I'm not qualified to review all the non-trivial C++ here, but aside from the above I don't see any mistakes in the Unixy bits.

review: Approve
Revision history for this message
Colin Watson (cjwatson) wrote :

And yes, the packaging changes look fine to me, so core-dev ack on that. The silo will need to contain all of dbus-cpp, unity-mir, and unity-scopes-api; and possibly also connectivity-api, platform-api, and qtubuntu-sensors (why do these build-depend on libprocess-cpp-dev but have no run-time dependency? That seems suspicious).

lp:~thomas-voss/process-cpp/fix-death-observer updated
53. By Thomas Voß

Make sure that the eventfd used by the SignalTrap is closed on destruction.
Make sure that signal blocks are setup for the correct scope.

Revision history for this message
Thomas Voß (thomas-voss) wrote :

> Nothing seems to close the event_fd at any point.
>

Fixed, thanks for catching it :)

> I'm not qualified to review all the non-trivial C++ here, but aside from the
> above I don't see any mistakes in the Unixy bits.

Revision history for this message
Thomas Voß (thomas-voss) wrote :

> And yes, the packaging changes look fine to me, so core-dev ack on that. The
> silo will need to contain all of dbus-cpp, unity-mir, and unity-scopes-api;

Yup, preparing.

> and possibly also connectivity-api, platform-api, and qtubuntu-sensors (why do
> these build-depend on libprocess-cpp-dev but have no run-time dependency?
> That seems suspicious).

These packages only leverage the out-of-process testing capabilities of process-cpp at buildtime.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:53
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/56/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/56
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/56
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/56/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/52

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/56/rebuild

review: Approve (continuous-integration)
lp:~thomas-voss/process-cpp/fix-death-observer updated
54. By Thomas Voß

Add missing symbol for ppc64el.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:54
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/57/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/57
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/57
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/57/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/53

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/57/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2014-02-04 10:22:50 +0000
3+++ CMakeLists.txt 2014-03-26 13:52:41 +0000
4@@ -56,9 +56,9 @@
5 SET(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -ftest-coverage -fprofile-arcs" )
6 ENDIF(cmake_build_type_lower MATCHES coverage)
7
8-set(PROCESS_CPP_VERSION_MAJOR 0)
9+set(PROCESS_CPP_VERSION_MAJOR 1)
10 set(PROCESS_CPP_VERSION_MINOR 0)
11-set(PROCESS_CPP_VERSION_PATCH 1)
12+set(PROCESS_CPP_VERSION_PATCH 0)
13
14 include(CTest)
15
16
17=== modified file 'debian/changelog'
18--- debian/changelog 2014-03-22 09:42:53 +0000
19+++ debian/changelog 2014-03-26 13:52:41 +0000
20@@ -1,3 +1,12 @@
21+process-cpp (1.0.0) UNRELEASED; urgency=medium
22+
23+ * Bump so name.
24+ * Fix death observer racyness.
25+ * Factor out signalfd-based wait operations on operating system signals.
26+ * Introduce interface SignalTrap and accompanying factory functions.
27+
28+ -- Thomas Voß <thomas.voss@canonical.com> Mon, 24 Mar 2014 18:06:46 +0100
29+
30 process-cpp (0.0.1+14.04.20140322-0ubuntu1) trusty; urgency=low
31
32 [ thomas-voss ]
33
34=== modified file 'debian/control'
35--- debian/control 2014-03-11 19:33:33 +0000
36+++ debian/control 2014-03-26 13:52:41 +0000
37@@ -19,7 +19,7 @@
38 Vcs-Bzr: https://code.launchpad.net/~phablet-team/content-hub/trunk
39 Vcs-Browser: https://bazaar.launchpad.net/~phablet-team/process-cpp/trunk/files
40
41-Package: libprocess-cpp0
42+Package: libprocess-cpp1
43 Architecture: any
44 Multi-Arch: same
45 Pre-Depends: ${misc:Pre-Depends},
46@@ -35,7 +35,7 @@
47 Architecture: any
48 Multi-Arch: same
49 Recommends: libprocess-cpp-doc,
50-Depends: libprocess-cpp0 (= ${binary:Version}),
51+Depends: libprocess-cpp1 (= ${binary:Version}),
52 ${misc:Depends},
53 libproperties-cpp-dev,
54 Description: C++11 library for handling processes - dev headers and libraries
55
56=== renamed file 'debian/libprocess-cpp0.install' => 'debian/libprocess-cpp1.install'
57=== renamed file 'debian/libprocess-cpp0.symbols' => 'debian/libprocess-cpp1.symbols'
58--- debian/libprocess-cpp0.symbols 2014-03-11 11:03:10 +0000
59+++ debian/libprocess-cpp1.symbols 2014-03-26 13:52:41 +0000
60@@ -1,76 +1,84 @@
61-libprocess-cpp.so.0 libprocess-cpp0 #MINVER#
62- (c++|arch=!amd64 !arm64 !ppc64el)"core::testing::CrossProcessSync::try_signal_ready_for(std::chrono::duration<long long, std::ratio<1ll, 1000ll> > const&)@Base" 0.0.1+14.04.20140311
63- (c++|arch=amd64 arm64 ppc64el)"core::testing::CrossProcessSync::try_signal_ready_for(std::chrono::duration<long, std::ratio<1l, 1000l> > const&)@Base" 0.0.1+14.04.20140311
64- (c++|arch=!amd64 !arm64 !ppc64el)"core::testing::CrossProcessSync::wait_for_signal_ready_for(std::chrono::duration<long long, std::ratio<1ll, 1000ll> > const&)@Base" 0.0.1+14.04.20140311
65- (c++|arch=amd64 arm64 ppc64el)"core::testing::CrossProcessSync::wait_for_signal_ready_for(std::chrono::duration<long, std::ratio<1l, 1000l> > const&)@Base" 0.0.1+14.04.20140311
66- (c++)"core::posix::ChildProcess::cerr()@Base" 0.0.1+14.04.20140311
67- (c++)"core::posix::ChildProcess::~ChildProcess()@Base" 0.0.1+14.04.20140311
68+libprocess-cpp.so.1 libprocess-cpp1 #MINVER#
69+ (c++)"core::posix::Signalable::send_signal(core::posix::Signal, std::error_code&)@Base" 0replaceme
70+ (c++)"core::posix::Signalable::send_signal_or_throw(core::posix::Signal)@Base" 0replaceme
71+ (c++)"core::posix::ChildProcess::DeathObserver::create_once_with_signal_trap(std::shared_ptr<core::posix::SignalTrap>)@Base" 0replaceme
72 (c++)"core::posix::ChildProcess::cin()@Base" 0.0.1+14.04.20140311
73+ (c++)"core::posix::ChildProcess::cerr()@Base" 0replaceme
74 (c++)"core::posix::ChildProcess::cout()@Base" 0.0.1+14.04.20140311
75- (c++)"core::posix::ChildProcess::DeathObserver::instance()@Base" 0.0.1+14.04.20140311
76 (c++)"core::posix::ChildProcess::invalid()@Base" 0.0.1+14.04.20140311
77 (c++)"core::posix::ChildProcess::wait_for(core::posix::wait::Flags const&)@Base" 0.0.1+14.04.20140311
78+ (c++)"core::posix::ChildProcess::~ChildProcess()@Base" 0replaceme
79+ (c++|arch=ppc64el)"core::posix::ChildProcess::ChildProcess(core::posix::ChildProcess const&)@Base" 0replaceme
80+ (c++)"core::posix::this_process::cin()@Base" 0replaceme
81+ (c++)"core::posix::this_process::env::get_or_throw(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 0replaceme
82+ (c++)"core::posix::this_process::env::set_or_throw(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 0replaceme
83+ (c++)"core::posix::this_process::env::unset_or_throw(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 0replaceme
84+ (c++)"core::posix::this_process::env::get(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 0replaceme
85+ (c++)"core::posix::this_process::env::set(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::error_code&)@Base" 0replaceme
86+ (c++)"core::posix::this_process::env::unset(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::error_code&)@Base" 0replaceme
87+ (c++)"core::posix::this_process::env::for_each(std::function<void (std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)> const&)@Base" 0replaceme
88+ (c++)"core::posix::this_process::cerr()@Base" 0replaceme
89+ (c++)"core::posix::this_process::cout()@Base" 0replaceme
90+ (c++)"core::posix::this_process::parent()@Base" 0replaceme
91+ (c++)"core::posix::this_process::instance()@Base" 0replaceme
92+ (c++)"core::posix::trap_signals_for_process(std::initializer_list<core::posix::Signal>)@Base" 0replaceme
93+ (c++)"core::posix::trap_signals_for_all_subsequent_threads(std::initializer_list<core::posix::Signal>)@Base" 0replaceme
94 (c++)"core::posix::exec(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, std::map<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const&, core::posix::StandardStream const&)@Base" 0.0.1+14.04.20140311
95 (c++)"core::posix::fork(std::function<core::posix::exit::Status ()> const&, core::posix::StandardStream const&)@Base" 0.0.1+14.04.20140311
96+ (c++)"core::posix::wait::operator|(core::posix::wait::Flags, core::posix::wait::Flags)@Base" 0replaceme
97+ (c++)"core::posix::linux::proc::process::OomScoreAdj::max_value()@Base" 0replaceme
98+ (c++)"core::posix::linux::proc::process::OomScoreAdj::min_value()@Base" 0replaceme
99 (c++)"core::posix::linux::proc::process::OomAdj::disable_value()@Base" 0.0.1+14.04.20140311
100 (c++)"core::posix::linux::proc::process::OomAdj::max_value()@Base" 0.0.1+14.04.20140311
101 (c++)"core::posix::linux::proc::process::OomAdj::min_value()@Base" 0.0.1+14.04.20140311
102- (c++)"core::posix::linux::proc::process::OomScoreAdj::max_value()@Base" 0.0.1+14.04.20140311
103- (c++)"core::posix::linux::proc::process::OomScoreAdj::min_value()@Base" 0.0.1+14.04.20140311
104- (c++)"core::posix::linux::proc::process::operator>>(core::posix::Process const&, core::posix::linux::proc::process::OomAdj&)@Base" 0.0.1+14.04.20140311
105+ (c++)"core::posix::linux::proc::process::operator<<(core::posix::Process const&, core::posix::linux::proc::process::OomScoreAdj const&)@Base" 0replaceme
106 (c++)"core::posix::linux::proc::process::operator<<(core::posix::Process const&, core::posix::linux::proc::process::OomAdj const&)@Base" 0.0.1+14.04.20140311
107 (c++)"core::posix::linux::proc::process::operator>>(core::posix::Process const&, core::posix::linux::proc::process::OomScoreAdj&)@Base" 0.0.1+14.04.20140311
108- (c++)"core::posix::linux::proc::process::operator<<(core::posix::Process const&, core::posix::linux::proc::process::OomScoreAdj const&)@Base" 0.0.1+14.04.20140311
109- (c++)"core::posix::linux::proc::process::operator>>(core::posix::Process const&, core::posix::linux::proc::process::OomScore&)@Base" 0.0.1+14.04.20140311
110 (c++)"core::posix::linux::proc::process::operator>>(core::posix::Process const&, core::posix::linux::proc::process::Stat&)@Base" 0.0.1+14.04.20140311
111- (c++)"core::posix::operator|(core::posix::StandardStream, core::posix::StandardStream)@Base" 0.0.1+14.04.20140311
112- (c++)"core::posix::operator&(core::posix::StandardStream, core::posix::StandardStream)@Base" 0.0.1+14.04.20140311
113- (c++)"core::posix::ProcessGroup::id() const@Base" 0.0.1+14.04.20140311
114+ (c++)"core::posix::linux::proc::process::operator>>(core::posix::Process const&, core::posix::linux::proc::process::OomAdj&)@Base" 0replaceme
115+ (c++)"core::posix::linux::proc::process::operator>>(core::posix::Process const&, core::posix::linux::proc::process::OomScore&)@Base" 0replaceme
116+ (c++)"core::posix::vfork(std::function<core::posix::exit::Status ()> const&, core::posix::StandardStream const&)@Base" 0replaceme
117 (c++)"core::posix::Process::invalid()@Base" 0.0.1+14.04.20140311
118- (c++)"core::posix::Process::pid() const@Base" 0.0.1+14.04.20140311
119- (c++)"core::posix::Process::~Process()@Base" 0.0.1+14.04.20140311
120- (c++)"core::posix::Process::process_group_or_throw() const@Base" 0.0.1+14.04.20140311
121- (c++)"core::posix::Process::process_group(std::error_code&) const@Base" 0.0.1+14.04.20140311
122 (c++)"core::posix::Process::Process(int)@Base" 0.0.1+14.04.20140311
123- (c++)"core::posix::Signalable::send_signal(core::posix::Signal, std::error_code&)@Base" 0.0.1+14.04.20140311
124- (c++)"core::posix::Signalable::send_signal_or_throw(core::posix::Signal)@Base" 0.0.1+14.04.20140311
125- (c++)"core::posix::this_process::cerr()@Base" 0.0.1+14.04.20140311
126- (c++)"core::posix::this_process::cin()@Base" 0.0.1+14.04.20140311
127- (c++)"core::posix::this_process::cout()@Base" 0.0.1+14.04.20140311
128- (c++)"core::posix::this_process::env::for_each(std::function<void (std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)> const&)@Base" 0.0.1+14.04.20140311
129- (c++)"core::posix::this_process::env::get_or_throw(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 0.0.1+14.04.20140311
130- (c++)"core::posix::this_process::env::get(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 0.0.1+14.04.20140311
131- (c++)"core::posix::this_process::env::set_or_throw(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 0.0.1+14.04.20140311
132- (c++)"core::posix::this_process::env::set(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::error_code&)@Base" 0.0.1+14.04.20140311
133- (c++)"core::posix::this_process::env::unset_or_throw(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 0.0.1+14.04.20140311
134- (c++)"core::posix::this_process::env::unset(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::error_code&)@Base" 0.0.1+14.04.20140311
135- (c++)"core::posix::this_process::instance()@Base" 0.0.1+14.04.20140311
136- (c++)"core::posix::this_process::parent()@Base" 0.0.1+14.04.20140311
137- (c++)"core::posix::vfork(std::function<core::posix::exit::Status ()> const&, core::posix::StandardStream const&)@Base" 0.0.1+14.04.20140311
138- (c++)"core::posix::wait::operator|(core::posix::wait::Flags, core::posix::wait::Flags)@Base" 0.0.1+14.04.20140311
139- (c++)"core::testing::CrossProcessSync::~CrossProcessSync()@Base" 0.0.1+14.04.20140311
140- (c++)"core::testing::CrossProcessSync::CrossProcessSync()@Base" 0.0.1+14.04.20140311
141+ (c++)"core::posix::Process::~Process()@Base" 0replaceme
142+ (c++)"core::posix::operator&(core::posix::StandardStream, core::posix::StandardStream)@Base" 0replaceme
143+ (c++)"core::posix::operator|(core::posix::StandardStream, core::posix::StandardStream)@Base" 0replaceme
144+ (c++)"core::testing::fork_and_run(std::function<core::posix::exit::Status ()> const&, std::function<core::posix::exit::Status ()> const&)@Base" 0replaceme
145+ (c++|arch=!amd64 !arm64 !ppc64el)"core::testing::CrossProcessSync::try_signal_ready_for(std::chrono::duration<long long, std::ratio<1ll, 1000ll> > const&)@Base" 0.0.1+14.04.20140311
146+ (c++|arch=amd64 arm64 ppc64el)"core::testing::CrossProcessSync::try_signal_ready_for(std::chrono::duration<long, std::ratio<1l, 1000l> > const&)@Base" 0.0.1+14.04.20140311
147+ (c++|arch=!amd64 !arm64 !ppc64el)"core::testing::CrossProcessSync::wait_for_signal_ready_for(std::chrono::duration<long long, std::ratio<1ll, 1000ll> > const&)@Base" 0.0.1+14.04.20140311
148+ (c++|arch=amd64 arm64 ppc64el)"core::testing::CrossProcessSync::wait_for_signal_ready_for(std::chrono::duration<long, std::ratio<1l, 1000l> > const&)@Base" 0.0.1+14.04.20140311
149+ (c++)"core::testing::CrossProcessSync::Error::Timeout::~Timeout()@Base" 0replaceme
150+ (c++)"core::testing::CrossProcessSync::CrossProcessSync(core::testing::CrossProcessSync const&)@Base" 0replaceme
151+ (c++)"core::testing::CrossProcessSync::CrossProcessSync()@Base" 0replaceme
152 (c++)"core::testing::CrossProcessSync::CrossProcessSync(core::testing::CrossProcessSync const&)@Base" 0.0.1+14.04.20140311
153- (c++)"core::testing::CrossProcessSync::Error::Timeout::~Timeout()@Base" 0.0.1+14.04.20140311
154+ (c++)"core::testing::CrossProcessSync::CrossProcessSync()@Base" 0replaceme
155+ (c++)"core::testing::CrossProcessSync::~CrossProcessSync()@Base" 0replaceme
156 (c++)"core::testing::CrossProcessSync::operator=(core::testing::CrossProcessSync const&)@Base" 0.0.1+14.04.20140311
157- (c++)"core::testing::fork_and_run(std::function<core::posix::exit::Status ()> const&, std::function<core::posix::exit::Status ()> const&)@Base" 0.0.1+14.04.20140311
158- (c++)"core::testing::operator|(core::testing::ForkAndRunResult, core::testing::ForkAndRunResult)@Base" 0.0.1+14.04.20140311
159 (c++)"core::testing::operator&(core::testing::ForkAndRunResult, core::testing::ForkAndRunResult)@Base" 0.0.1+14.04.20140311
160- (c++)"typeinfo for core::posix::ChildProcess@Base" 0.0.1+14.04.20140311
161+ (c++)"core::testing::operator|(core::testing::ForkAndRunResult, core::testing::ForkAndRunResult)@Base" 0replaceme
162+ (c++)"core::posix::ProcessGroup::id() const@Base" 0replaceme
163+ (c++)"core::posix::Process::process_group(std::error_code&) const@Base" 0replaceme
164+ (c++)"core::posix::Process::process_group_or_throw() const@Base" 0replaceme
165+ (c++)"core::posix::Process::pid() const@Base" 0replaceme
166+ (c++)"typeinfo for core::posix::SignalTrap@Base" 0replaceme
167+ (c++)"typeinfo for core::posix::Signalable@Base" 0replaceme
168 (c++)"typeinfo for core::posix::ChildProcess::DeathObserver@Base" 0.0.1+14.04.20140311
169- (c++)"typeinfo for core::posix::Process@Base" 0.0.1+14.04.20140311
170+ (c++)"typeinfo for core::posix::ChildProcess@Base" 0replaceme
171 (c++)"typeinfo for core::posix::ProcessGroup@Base" 0.0.1+14.04.20140311
172- (c++)"typeinfo for core::posix::Signalable@Base" 0.0.1+14.04.20140311
173+ (c++)"typeinfo for core::posix::Process@Base" 0replaceme
174 (c++)"typeinfo for core::testing::CrossProcessSync::Error::Timeout@Base" 0.0.1+14.04.20140311
175- (c++)"typeinfo name for core::posix::ChildProcess@Base" 0.0.1+14.04.20140311
176+ (c++)"typeinfo name for core::posix::SignalTrap@Base" 0replaceme
177+ (c++)"typeinfo name for core::posix::Signalable@Base" 0replaceme
178 (c++)"typeinfo name for core::posix::ChildProcess::DeathObserver@Base" 0.0.1+14.04.20140311
179- (c++)"typeinfo name for core::posix::Process@Base" 0.0.1+14.04.20140311
180+ (c++)"typeinfo name for core::posix::ChildProcess@Base" 0replaceme
181 (c++)"typeinfo name for core::posix::ProcessGroup@Base" 0.0.1+14.04.20140311
182- (c++)"typeinfo name for core::posix::Signalable@Base" 0.0.1+14.04.20140311
183+ (c++)"typeinfo name for core::posix::Process@Base" 0replaceme
184 (c++)"typeinfo name for core::testing::CrossProcessSync::Error::Timeout@Base" 0.0.1+14.04.20140311
185- (c++)"vtable for core::posix::ChildProcess@Base" 0.0.1+14.04.20140311
186+ (c++)"vtable for core::posix::SignalTrap@Base" 0replaceme
187+ (c++)"vtable for core::posix::Signalable@Base" 0replaceme
188 (c++)"vtable for core::posix::ChildProcess::DeathObserver@Base" 0.0.1+14.04.20140311
189- (c++)"vtable for core::posix::Process@Base" 0.0.1+14.04.20140311
190+ (c++)"vtable for core::posix::ChildProcess@Base" 0replaceme
191 (c++)"vtable for core::posix::ProcessGroup@Base" 0.0.1+14.04.20140311
192- (c++)"vtable for core::posix::Signalable@Base" 0.0.1+14.04.20140311
193+ (c++)"vtable for core::posix::Process@Base" 0replaceme
194 (c++)"vtable for core::testing::CrossProcessSync::Error::Timeout@Base" 0.0.1+14.04.20140311
195
196=== modified file 'include/core/posix/child_process.h'
197--- include/core/posix/child_process.h 2014-02-10 20:02:48 +0000
198+++ include/core/posix/child_process.h 2014-03-26 13:52:41 +0000
199@@ -57,9 +57,12 @@
200 {
201 public:
202 /**
203- * @brief Access the single instance of the death observer.
204+ * @brief Creates the unique instance of class DeathObserver.
205+ * @throw std::logic_error if the given SignalTrap instance does not trap Signal::sig_chld.
206+ * @throw std::runtime_error if there already is an instance of the death observer.
207 */
208- static DeathObserver& instance();
209+ static std::unique_ptr<DeathObserver> create_once_with_signal_trap(
210+ std::shared_ptr<SignalTrap> trap);
211
212 DeathObserver(const DeathObserver&) = delete;
213 virtual ~DeathObserver() = default;
214@@ -87,16 +90,9 @@
215 virtual const core::Signal<ChildProcess>& child_died() const = 0;
216
217 /**
218- * @brief run starts the observer and blocks until quit() is called.
219- * @throw std::runtime_error if called more than once without intermediate quit().
220- * @param ec Destination to store errors.
221- */
222- virtual void run(std::error_code& ec) = 0;
223-
224- /**
225- * @brief quit stops the execution of the observer.
226- */
227- virtual void quit() = 0;
228+ * @brief Checks and reaps all child processes registered with the observer instance.
229+ */
230+ virtual void on_sig_child() = 0;
231
232 protected:
233 DeathObserver() = default;
234
235=== modified file 'include/core/posix/signal.h'
236--- include/core/posix/signal.h 2013-11-29 07:20:00 +0000
237+++ include/core/posix/signal.h 2014-03-26 13:52:41 +0000
238@@ -19,8 +19,15 @@
239 #ifndef CORE_POSIX_SIGNAL_H_
240 #define CORE_POSIX_SIGNAL_H_
241
242+#include <core/posix/visibility.h>
243+
244+#include <core/signal.h>
245+
246 #include <signal.h>
247
248+#include <initializer_list>
249+#include <memory>
250+
251 namespace core
252 {
253 namespace posix
254@@ -30,6 +37,7 @@
255 */
256 enum class Signal
257 {
258+ unknown = 0,
259 sig_hup = SIGHUP,
260 sig_int = SIGINT,
261 sig_quit = SIGQUIT,
262@@ -50,6 +58,59 @@
263 sig_ttin = SIGTTIN,
264 sig_ttou = SIGTTOU
265 };
266+
267+/**
268+ * @brief The SignalTrap class encapsulates functionality to trap and handle signals.
269+ */
270+class CORE_POSIX_DLL_PUBLIC SignalTrap
271+{
272+public:
273+ SignalTrap(const SignalTrap&) = delete;
274+ virtual ~SignalTrap() = default;
275+
276+ SignalTrap& operator=(const SignalTrap&) = delete;
277+ bool operator==(const SignalTrap&) const = delete;
278+
279+ /**
280+ * @brief Returns true if the given signal is trapped by this instance.
281+ */
282+ virtual bool has(Signal signal) = 0;
283+
284+ /**
285+ * @brief Starts observation of incoming signals, relaying them via
286+ * signal_raised(). The call blocks until stop is called.
287+ */
288+ virtual void run() = 0;
289+
290+ /**
291+ * @brief Stops execution of the signal trap.
292+ */
293+ virtual void stop() = 0;
294+
295+ /**
296+ * @brief Emitted whenever a trapped signal is raised by the operating system.
297+ */
298+ virtual core::Signal<Signal>& signal_raised() = 0;
299+
300+protected:
301+ SignalTrap() = default;
302+};
303+
304+/**
305+ * @brief Traps the specified signals for the entire process.
306+ */
307+CORE_POSIX_DLL_PUBLIC
308+std::shared_ptr<SignalTrap> trap_signals_for_process(
309+ std::initializer_list<core::posix::Signal> blocked_signals);
310+
311+/**
312+ * @brief Traps the specified signals for the current thread, and inherits
313+ * the respective signal mask to all child-threads.
314+ */
315+CORE_POSIX_DLL_PUBLIC
316+std::shared_ptr<SignalTrap> trap_signals_for_all_subsequent_threads(
317+ std::initializer_list<core::posix::Signal> blocked_signals);
318+
319 }
320 }
321
322
323=== modified file 'src/CMakeLists.txt'
324--- src/CMakeLists.txt 2014-03-06 11:56:25 +0000
325+++ src/CMakeLists.txt 2014-03-26 13:52:41 +0000
326@@ -25,6 +25,7 @@
327 core/posix/fork.cpp
328 core/posix/process.cpp
329 core/posix/process_group.cpp
330+ core/posix/signal.cpp
331 core/posix/signalable.cpp
332 core/posix/standard_stream.cpp
333 core/posix/wait.cpp
334
335=== modified file 'src/core/posix/child_process.cpp'
336--- src/core/posix/child_process.cpp 2014-03-14 14:35:51 +0000
337+++ src/core/posix/child_process.cpp 2014-03-26 13:52:41 +0000
338@@ -37,42 +37,29 @@
339
340 namespace
341 {
342-struct SignalFdDeathObserver : public core::posix::ChildProcess::DeathObserver
343+
344+struct DeathObserverImpl : public core::posix::ChildProcess::DeathObserver
345 {
346- enum class State
347- {
348- not_running,
349- running
350- };
351-
352- SignalFdDeathObserver()
353- : signal_fd(-1),
354- wakeup_fd(-1),
355- state(State::not_running)
356- {
357- ::sigemptyset(&mask);
358- ::sigaddset(&mask, SIGCHLD);
359-
360- if (::sigprocmask(SIG_BLOCK, &mask, &old_process_mask) == -1)
361- throw std::system_error(errno, std::system_category());
362-
363- signal_fd = ::signalfd(signal_fd, &mask, SFD_NONBLOCK);
364-
365- if (signal_fd == -1)
366- throw std::system_error(errno, std::system_category());
367-
368- static const unsigned int initial_value = 0;
369- wakeup_fd = ::eventfd(initial_value, EFD_NONBLOCK);
370-
371- if (wakeup_fd == -1)
372- throw std::system_error(errno, std::system_category());
373- }
374-
375- ~SignalFdDeathObserver()
376- {
377- ::sigprocmask(SIG_SETMASK, &old_process_mask, nullptr);
378- ::close(signal_fd);
379- ::close(wakeup_fd);
380+ DeathObserverImpl(const std::shared_ptr<core::posix::SignalTrap>& trap)
381+ : on_sig_child_connection
382+ {
383+ trap->signal_raised().connect([this](core::posix::Signal signal)
384+ {
385+ switch (signal)
386+ {
387+ case core::posix::Signal::sig_chld:
388+ on_sig_child();
389+ break;
390+ default:
391+ break;
392+ }
393+ })
394+ }
395+ {
396+ if (!trap->has(core::posix::Signal::sig_chld))
397+ throw std::logic_error(
398+ "DeathObserver::DeathObserverImpl: Given SignalTrap"
399+ " instance does not trap Signal::sig_chld.");
400 }
401
402 bool add(const core::posix::ChildProcess& process) override
403@@ -115,132 +102,45 @@
404 return signals.child_died;
405 }
406
407- void run(std::error_code& ec) override
408+ void on_sig_child() override
409 {
410- if (state.load() == State::running)
411- throw std::runtime_error("DeathObserver::run can only be run once.");
412-
413- state.store(State::running);
414-
415- sigset_t old_mask;
416-
417- if (::pthread_sigmask(SIG_SETMASK, &mask, &old_mask) == -1)
418- {
419- ec = std::error_code(errno, std::system_category());
420- return;
421- }
422-
423- signalfd_siginfo signal_info;
424- ::memset(&signal_info, 0, sizeof(signal_info));
425-
426- static const int signal_fd_idx = 0;
427- static const int wakeup_fd_idx = 1;
428-
429- pollfd fds[2];
430- fds[0] = {signal_fd, POLLIN, 0};
431- fds[1] = {wakeup_fd, POLLIN, 0};
432-
433+ pid_t pid{-1}; int status{-1};
434 while (true)
435 {
436- auto rc = ::poll(fds, 2, -1);
437-
438- if (rc == -1)
439- {
440- if (errno == EINTR)
441- continue;
442-
443- ec = std::error_code(errno, std::system_category());
444- break;
445- }
446-
447- if (rc == 0)
448- continue;
449-
450- if (fds[signal_fd_idx].revents == POLLIN)
451- {
452- auto result = ::read(signal_fd, &signal_info, sizeof(signal_info));
453- if (result == -1 || result != sizeof(signal_info))
454- {
455- ec = std::error_code(errno, std::system_category());
456- }
457- else if (result == sizeof(signal_info))
458- {
459- switch(signal_info.ssi_signo)
460- {
461- case SIGCHLD:
462- {
463- pid_t pid{-1}; int status{-1};
464- while (true)
465- {
466- pid = ::waitpid(-1, &status, WNOHANG);
467-
468- if (pid == -1)
469- {
470- if (errno == ECHILD)
471- {
472- break; // No more children
473- }
474- continue; // Ignore stray SIGCHLD signals
475- }
476- else if (pid == 0)
477- {
478- break; // No more children
479- }
480- else
481- {
482- std::lock_guard<std::mutex> lg(guard);
483- auto it = children.find(pid);
484-
485- if (it != children.end())
486- {
487- if (WIFSIGNALED(status) || WIFEXITED(status))
488- {
489- signals.child_died(it->second);
490- children.erase(it);
491- }
492- }
493- }
494- }
495- break;
496- }
497- default:
498- break;
499- }
500- }
501- }
502-
503- if (fds[wakeup_fd_idx].revents == POLLIN)
504- {
505- std::int64_t value{1};
506- auto result = ::read(wakeup_fd, &value, sizeof(value));
507- if (result == -1 || result != sizeof(value))
508- {
509- ec = std::error_code(errno, std::system_category());
510- }
511- break;
512+ pid = ::waitpid(-1, &status, WNOHANG);
513+
514+ if (pid == -1)
515+ {
516+ if (errno == ECHILD)
517+ {
518+ break; // No more children
519+ }
520+ continue; // Ignore stray SIGCHLD signals
521+ }
522+ else if (pid == 0)
523+ {
524+ break; // No more children
525+ }
526+ else
527+ {
528+ std::lock_guard<std::mutex> lg(guard);
529+ auto it = children.find(pid);
530+
531+ if (it != children.end())
532+ {
533+ if (WIFSIGNALED(status) || WIFEXITED(status))
534+ {
535+ signals.child_died(it->second);
536+ children.erase(it);
537+ }
538+ }
539 }
540 }
541-
542- if (pthread_sigmask(SIG_SETMASK, &old_mask, nullptr) == -1)
543- ec = std::error_code(errno, std::system_category());
544-
545- state.store(State::not_running);
546- }
547-
548- void quit() override
549- {
550- static const std::int64_t value = {1};
551- if (sizeof(value) != ::write(wakeup_fd, &value, sizeof(value)))
552- throw std::system_error(errno, std::system_category());
553- }
554-
555- sigset_t mask;
556- sigset_t old_process_mask;
557- int signal_fd;
558- int wakeup_fd;
559- std::atomic<State> state;
560+ }
561+
562 mutable std::mutex guard;
563 std::unordered_map<pid_t, core::posix::ChildProcess> children;
564+ core::ScopedConnection on_sig_child_connection;
565 struct
566 {
567 core::Signal<core::posix::ChildProcess> child_died;
568@@ -248,16 +148,46 @@
569 };
570 }
571
572+std::unique_ptr<core::posix::ChildProcess::DeathObserver>
573+core::posix::ChildProcess::DeathObserver::create_once_with_signal_trap(
574+ std::shared_ptr<core::posix::SignalTrap> trap)
575+{
576+ static std::atomic<bool> has_been_created_once{false};
577+
578+ if (has_been_created_once.exchange(true))
579+ throw std::runtime_error
580+ {
581+ "DeathObserver::create_once_with_signal_trap: "
582+ "Cannot create more than one instance."
583+ };
584+
585+ try
586+ {
587+ std::unique_ptr<core::posix::ChildProcess::DeathObserver> result
588+ {
589+ new DeathObserverImpl{trap}
590+ };
591+
592+ return result;
593+ } catch(...)
594+ {
595+ // We make sure that a throwing c'tor does not impact our ability to
596+ // retry creation of a DeathObserver instance.
597+ has_been_created_once.store(false);
598+
599+ std::rethrow_exception(std::current_exception());
600+ }
601+
602+ assert(false && "We should never reach here.");
603+
604+ // Silence the compiler.
605+ return std::unique_ptr<core::posix::ChildProcess::DeathObserver>{};
606+}
607+
608 namespace core
609 {
610 namespace posix
611 {
612-ChildProcess::DeathObserver& ChildProcess::DeathObserver::instance()
613-{
614- static SignalFdDeathObserver observer;
615- return observer;
616-}
617-
618 ChildProcess::Pipe ChildProcess::Pipe::invalid()
619 {
620 static Pipe p;
621
622=== added file 'src/core/posix/signal.cpp'
623--- src/core/posix/signal.cpp 1970-01-01 00:00:00 +0000
624+++ src/core/posix/signal.cpp 2014-03-26 13:52:41 +0000
625@@ -0,0 +1,217 @@
626+/*
627+ * Copyright © 2013 Canonical Ltd.
628+ *
629+ * This program is free software: you can redistribute it and/or modify it
630+ * under the terms of the GNU Lesser General Public License version 3,
631+ * as published by the Free Software Foundation.
632+ *
633+ * This program is distributed in the hope that it will be useful,
634+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
635+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
636+ * GNU Lesser General Public License for more details.
637+ *
638+ * You should have received a copy of the GNU Lesser General Public License
639+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
640+ *
641+ * Authored by: Thomas Voß <thomas.voss@canonical.com>
642+ */
643+
644+#include <core/posix/signalable.h>
645+
646+#include <poll.h>
647+#include <pthread.h>
648+#include <sys/eventfd.h>
649+#include <sys/signalfd.h>
650+
651+#include <unistd.h>
652+
653+#include <atomic>
654+
655+namespace impl
656+{
657+void set_thread_signal_mask(::sigset_t* new_mask, ::sigset_t* old_mask)
658+{
659+ ::pthread_sigmask(SIG_BLOCK, new_mask, old_mask);
660+}
661+
662+void set_process_signal_mask(::sigset_t* new_mask, ::sigset_t* old_mask)
663+{
664+ ::sigprocmask(SIG_BLOCK, new_mask, old_mask);
665+}
666+
667+class SignalTrap : public core::posix::SignalTrap
668+{
669+public:
670+ enum class Scope
671+ {
672+ process,
673+ thread
674+ };
675+
676+ enum class State
677+ {
678+ not_running,
679+ running
680+ };
681+
682+ SignalTrap(Scope scope, std::initializer_list<core::posix::Signal> blocked_signals)
683+ : scope(scope),
684+ state(State::not_running),
685+ event_fd(::eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK))
686+ {
687+ if (event_fd == -1)
688+ throw std::system_error(errno, std::system_category());
689+
690+ ::sigemptyset(&blocked_signals_mask);
691+
692+ for(auto signal : blocked_signals)
693+ ::sigaddset(&blocked_signals_mask, static_cast<int>(signal));
694+
695+ switch (scope)
696+ {
697+ case Scope::process:
698+ set_process_signal_mask(&blocked_signals_mask, &old_signals_mask);
699+ break;
700+ case Scope::thread:
701+ set_thread_signal_mask(&blocked_signals_mask, &old_signals_mask);
702+ break;
703+ }
704+ }
705+
706+ ~SignalTrap()
707+ {
708+ switch (scope)
709+ {
710+ case Scope::process:
711+ set_process_signal_mask(&old_signals_mask, nullptr);
712+ break;
713+ case Scope::thread:
714+ set_thread_signal_mask(&old_signals_mask, nullptr);
715+ break;
716+ }
717+
718+ ::close(event_fd);
719+ }
720+
721+ bool has(core::posix::Signal signal) override
722+ {
723+ return ::sigismember(&blocked_signals_mask, static_cast<int>(signal));
724+ }
725+
726+ void run() override
727+ {
728+ static constexpr int signal_fd_idx = 0;
729+ static constexpr int event_fd_idx = 1;
730+
731+ static constexpr int signal_info_buffer_size = 5;
732+
733+ if (state.load() == State::running)
734+ throw std::runtime_error("SignalTrap::run can only be run once.");
735+
736+ state.store(State::running);
737+
738+ // Make sure we clean up the signal fd whenever
739+ // we leave the scope of run.
740+ struct Scope
741+ {
742+ ~Scope()
743+ {
744+ if (signal_fd != -1)
745+ ::close(signal_fd);
746+ }
747+
748+ int signal_fd;
749+ } scope{::signalfd(-1, &blocked_signals_mask, SFD_CLOEXEC | SFD_NONBLOCK)};
750+
751+ if (scope.signal_fd == -1)
752+ throw std::system_error(errno, std::system_category());
753+
754+ pollfd fds[2];
755+ signalfd_siginfo signal_info[signal_info_buffer_size];
756+
757+ for (;;)
758+ {
759+ fds[signal_fd_idx] = {scope.signal_fd, POLLIN, 0};
760+ fds[event_fd_idx] = {event_fd, POLLIN, 0};
761+
762+ auto rc = ::poll(fds, 2, -1);
763+
764+ if (rc == -1)
765+ {
766+ if (errno == EINTR)
767+ continue;
768+
769+ break;
770+ }
771+
772+ if (rc == 0)
773+ continue;
774+
775+ if (fds[signal_fd_idx].revents & POLLIN)
776+ {
777+ auto result = ::read(scope.signal_fd, signal_info, sizeof(signal_info));
778+
779+ for (uint i = 0; i < result / sizeof(signalfd_siginfo); i++)
780+ {
781+ if (has(static_cast<core::posix::Signal>(signal_info[i].ssi_signo)))
782+ {
783+ on_signal_raised(
784+ static_cast<core::posix::Signal>(
785+ signal_info[i].ssi_signo));
786+ }
787+ }
788+ }
789+
790+ if (fds[event_fd_idx].revents & POLLIN)
791+ {
792+ std::int64_t value{1};
793+ // Consciously void-ing the return value here.
794+ // Not much we can do about an error.
795+ auto result = ::read(event_fd, &value, sizeof(value));
796+ (void) result;
797+
798+ break;
799+ }
800+ }
801+
802+ state.store(State::not_running);
803+ }
804+
805+ void stop() override
806+ {
807+ static const std::int64_t value = {1};
808+ if (sizeof(value) != ::write(event_fd, &value, sizeof(value)))
809+ throw std::system_error(errno, std::system_category());
810+ }
811+
812+ core::Signal<core::posix::Signal>& signal_raised() override
813+ {
814+ return on_signal_raised;
815+ }
816+
817+private:
818+ Scope scope;
819+ std::atomic<State> state;
820+ int event_fd;
821+ core::Signal<core::posix::Signal> on_signal_raised;
822+ ::sigset_t old_signals_mask;
823+ ::sigset_t blocked_signals_mask;
824+};
825+}
826+
827+std::shared_ptr<core::posix::SignalTrap> core::posix::trap_signals_for_process(
828+ std::initializer_list<core::posix::Signal> blocked_signals)
829+{
830+ return std::make_shared<impl::SignalTrap>(
831+ impl::SignalTrap::Scope::process,
832+ blocked_signals);
833+}
834+
835+std::shared_ptr<core::posix::SignalTrap> core::posix::trap_signals_for_all_subsequent_threads(
836+ std::initializer_list<core::posix::Signal> blocked_signals)
837+{
838+ return std::make_shared<impl::SignalTrap>(
839+ impl::SignalTrap::Scope::thread,
840+ blocked_signals);
841+}
842+
843
844=== modified file 'tests/CMakeLists.txt'
845--- tests/CMakeLists.txt 2014-02-04 10:22:50 +0000
846+++ tests/CMakeLists.txt 2014-03-26 13:52:41 +0000
847@@ -57,6 +57,11 @@
848 cross_process_sync_test.cpp
849 )
850
851+add_executable(
852+ death_observer_test
853+ death_observer_test.cpp
854+)
855+
856 target_link_libraries(
857 posix_process_test
858
859@@ -93,7 +98,17 @@
860 ${GMOCK_BOTH_LIBRARIES}
861 )
862
863+target_link_libraries(
864+ death_observer_test
865+
866+ process-cpp
867+
868+ ${CMAKE_THREAD_LIBS_INIT}
869+ ${GMOCK_BOTH_LIBRARIES}
870+)
871+
872 add_test(posix_process_test ${CMAKE_CURRENT_BINARY_DIR}/posix_process_test)
873 add_test(linux_process_test ${CMAKE_CURRENT_BINARY_DIR}/linux_process_test)
874 add_test(fork_and_run_test ${CMAKE_CURRENT_BINARY_DIR}/fork_and_run_test)
875 add_test(cross_process_sync_test ${CMAKE_CURRENT_BINARY_DIR}/cross_process_sync_test)
876+add_test(death_observer_test ${CMAKE_CURRENT_BINARY_DIR}/death_observer_test)
877
878=== added file 'tests/death_observer_test.cpp'
879--- tests/death_observer_test.cpp 1970-01-01 00:00:00 +0000
880+++ tests/death_observer_test.cpp 2014-03-26 13:52:41 +0000
881@@ -0,0 +1,36 @@
882+/*
883+ * Copyright © 2013 Canonical Ltd.
884+ *
885+ * This program is free software: you can redistribute it and/or modify it
886+ * under the terms of the GNU Lesser General Public License version 3,
887+ * as published by the Free Software Foundation.
888+ *
889+ * This program is distributed in the hope that it will be useful,
890+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
891+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
892+ * GNU Lesser General Public License for more details.
893+ *
894+ * You should have received a copy of the GNU Lesser General Public License
895+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
896+ *
897+ * Authored by: Thomas Voß <thomas.voss@canonical.com>
898+ */
899+
900+#include <core/posix/child_process.h>
901+#include <core/posix/signal.h>
902+
903+#include <core/testing/fork_and_run.h>
904+
905+#include <gtest/gtest.h>
906+
907+TESTP(DeathObserver, construction_and_deconstruction_works,
908+{
909+ auto trap = core::posix::trap_signals_for_all_subsequent_threads({core::posix::Signal::sig_chld});
910+ EXPECT_NO_THROW(auto death_observer = core::posix::ChildProcess::DeathObserver::create_once_with_signal_trap(trap));
911+})
912+
913+TESTP(DeathObserver, construction_with_a_trap_not_including_sig_chld_throws,
914+{
915+ auto trap = core::posix::trap_signals_for_all_subsequent_threads({core::posix::Signal::sig_term});
916+ EXPECT_ANY_THROW(auto death_observer = core::posix::ChildProcess::DeathObserver::create_once_with_signal_trap(trap));
917+})
918
919=== modified file 'tests/posix_process_test.cpp'
920--- tests/posix_process_test.cpp 2014-03-05 21:08:28 +0000
921+++ tests/posix_process_test.cpp 2014-03-26 13:52:41 +0000
922@@ -50,6 +50,23 @@
923
924 core::posix::ChildProcess child = core::posix::ChildProcess::invalid();
925 };
926+
927+struct Init
928+{
929+ Init()
930+ : signal_trap(
931+ core::posix::trap_signals_for_all_subsequent_threads(
932+ {core::posix::Signal::sig_chld})),
933+ death_observer(
934+ core::posix::ChildProcess::DeathObserver::create_once_with_signal_trap(
935+ signal_trap))
936+ {
937+ }
938+
939+ std::shared_ptr<core::posix::SignalTrap> signal_trap;
940+ std::unique_ptr<core::posix::ChildProcess::DeathObserver> death_observer;
941+} init;
942+
943 }
944
945 TEST(PosixProcess, ctor_throws_for_invalid_pid)
946@@ -344,7 +361,7 @@
947
948 namespace
949 {
950-struct ChildDeathObserverSignalTrap
951+struct ChildDeathObserverEventCollector
952 {
953 MOCK_METHOD1(on_child_died,void(const core::posix::Process&));
954 };
955@@ -354,51 +371,43 @@
956 {
957 using namespace ::testing;
958
959- ChildDeathObserverSignalTrap signal_trap;
960-
961- auto& death_observer = core::posix::ChildProcess::DeathObserver::instance();
962-
963- EXPECT_TRUE(death_observer.add(child));
964+ ChildDeathObserverEventCollector event_collector;
965
966 core::ScopedConnection sc
967 {
968- death_observer.child_died().connect([&signal_trap](const core::posix::ChildProcess& child)
969+ init.death_observer->child_died().connect([&event_collector](core::posix::ChildProcess cp)
970 {
971- signal_trap.on_child_died(child);
972+ event_collector.on_child_died(cp);
973 })
974 };
975
976- EXPECT_CALL(signal_trap, on_child_died(_))
977+ EXPECT_TRUE(init.death_observer->add(child));
978+ EXPECT_CALL(event_collector, on_child_died(_))
979 .Times(1)
980 .WillOnce(
981 InvokeWithoutArgs(
982- &death_observer,
983- &core::posix::ChildProcess::DeathObserver::quit));
984+ init.signal_trap.get(),
985+ &core::posix::SignalTrap::stop));
986
987- std::error_code ec;
988- std::thread worker{[&death_observer, &ec]() { death_observer.run(ec); }};
989+ std::thread worker{[]() { init.signal_trap->run(); }};
990
991 child.send_signal_or_throw(core::posix::Signal::sig_kill);
992
993 if (worker.joinable())
994 worker.join();
995-
996- EXPECT_FALSE(ec);
997 }
998
999 TEST_F(ForkedSpinningProcess, observing_child_processes_for_death_works_if_child_is_signalled_with_sigterm)
1000 {
1001 using namespace ::testing;
1002
1003- ChildDeathObserverSignalTrap signal_trap;
1004-
1005- auto& death_observer = core::posix::ChildProcess::DeathObserver::instance();
1006-
1007- EXPECT_TRUE(death_observer.add(child));
1008+ ChildDeathObserverEventCollector signal_trap;
1009+
1010+ EXPECT_TRUE(init.death_observer->add(child));
1011
1012 core::ScopedConnection sc
1013 {
1014- death_observer.child_died().connect([&signal_trap](const core::posix::ChildProcess& child)
1015+ init.death_observer->child_died().connect([&signal_trap](const core::posix::ChildProcess& child)
1016 {
1017 signal_trap.on_child_died(child);
1018 })
1019@@ -408,57 +417,51 @@
1020 .Times(1)
1021 .WillOnce(
1022 InvokeWithoutArgs(
1023- &death_observer,
1024- &core::posix::ChildProcess::DeathObserver::quit));
1025+ init.signal_trap.get(),
1026+ &core::posix::SignalTrap::stop));
1027
1028- std::error_code ec;
1029- std::thread worker{[&death_observer, &ec]() { death_observer.run(ec); }};
1030+ std::thread worker{[]() { init.signal_trap->run(); }};
1031
1032 child.send_signal_or_throw(core::posix::Signal::sig_term);
1033
1034 if (worker.joinable())
1035 worker.join();
1036-
1037- EXPECT_FALSE(ec);
1038 }
1039
1040 TEST(ChildProcess, ensure_that_forked_children_are_cleaned_up)
1041 {
1042 static const unsigned int child_process_count = 100;
1043- static unsigned int counter = 0;
1044+ unsigned int counter = 1;
1045
1046- auto& death_observer = core::posix::ChildProcess::DeathObserver::instance();
1047 core::ScopedConnection sc
1048 {
1049- death_observer.child_died().connect([&death_observer](const core::posix::ChildProcess&)
1050+ init.death_observer->child_died().connect([&counter](const core::posix::ChildProcess&)
1051 {
1052 counter++;
1053
1054 if (counter == child_process_count)
1055 {
1056- death_observer.quit();
1057+ init.signal_trap->stop();
1058 }
1059 })
1060 };
1061
1062- std::error_code ec;
1063- std::thread t([&death_observer, &ec]() {death_observer.run(ec);});
1064+ std::thread t([]() {init.signal_trap->run();});
1065
1066 for (unsigned int i = 0; i < child_process_count; i++)
1067 {
1068 auto child = core::posix::fork(
1069 []() { return core::posix::exit::Status::success; },
1070 core::posix::StandardStream::stdin | core::posix::StandardStream::stdout);
1071- death_observer.add(child);
1072+ init.death_observer->add(child);
1073 // A bit ugly but we have to ensure that no signal is lost.
1074 // And thus, we keep the process object alive.
1075- std::this_thread::sleep_for(std::chrono::milliseconds{10});
1076+ std::this_thread::sleep_for(std::chrono::milliseconds{5});
1077 }
1078
1079 if (t.joinable())
1080 t.join();
1081
1082- EXPECT_FALSE(ec);
1083 EXPECT_EQ(child_process_count, counter);
1084 }
1085

Subscribers

People subscribed via source and target branches