Replace custom synchronization with absl::Notification

PiperOrigin-RevId: 248334969
Change-Id: I7614a3792babd399912c5d5a167ab5e0a0574d20
This commit is contained in:
Wiktor Garbacz 2019-05-15 08:09:39 -07:00 committed by Copybara-Service
parent 42761c8b72
commit 7294e9976e
3 changed files with 15 additions and 21 deletions

View File

@ -112,7 +112,6 @@ Monitor::Monitor(Executor* executor, Policy* policy, Notify* notify)
policy_(policy), policy_(policy),
comms_(executor_->ipc()->comms()), comms_(executor_->ipc()->comms()),
ipc_(executor_->ipc()), ipc_(executor_->ipc()),
setup_counter_(1),
wait_for_execve_(executor->enable_sandboxing_pre_execve_) { wait_for_execve_(executor->enable_sandboxing_pre_execve_) {
std::string path = std::string path =
absl::GetFlag(FLAGS_sandbox2_danger_danger_permit_all_and_log); absl::GetFlag(FLAGS_sandbox2_danger_danger_permit_all_and_log);
@ -141,19 +140,17 @@ void LogContainer(const std::vector<std::string>& container) {
} // namespace } // namespace
void Monitor::Run() { void Monitor::Run() {
using DecrementCounter = decltype(setup_counter_); std::unique_ptr<absl::Notification, void (*)(absl::Notification*)>
std::unique_ptr<DecrementCounter, void (*)(DecrementCounter*)> setup_notify{&setup_notification_, [](absl::Notification* notification) {
decrement_count{&setup_counter_, [](DecrementCounter* counter) { notification->Notify();
counter->DecrementCount(); }};
}};
struct MonitorCleanup { struct MonitorCleanup {
~MonitorCleanup() { ~MonitorCleanup() {
getrusage(RUSAGE_THREAD, capture->result_.GetRUsageMonitor()); getrusage(RUSAGE_THREAD, capture->result_.GetRUsageMonitor());
capture->notify_->EventFinished(capture->result_); capture->notify_->EventFinished(capture->result_);
capture->ipc_->InternalCleanupFdMap(); capture->ipc_->InternalCleanupFdMap();
absl::MutexLock lock(&capture->done_mutex_); capture->done_notification_.Notify();
capture->done_.store(true, std::memory_order_release);
} }
Monitor* capture; Monitor* capture;
} monitor_cleanup{this}; } monitor_cleanup{this};
@ -243,7 +240,7 @@ void Monitor::Run() {
// Tell the parent thread (Sandbox2 object) that we're done with the initial // Tell the parent thread (Sandbox2 object) that we're done with the initial
// set-up process of the sandboxee. // set-up process of the sandboxee.
decrement_count.reset(); setup_notify.reset();
MainLoop(&sigtimedwait_sset); MainLoop(&sigtimedwait_sset);
} }

View File

@ -27,7 +27,7 @@
#include <ctime> #include <ctime>
#include <memory> #include <memory>
#include "absl/synchronization/blocking_counter.h" #include "absl/synchronization/notification.h"
#include "sandboxed_api/sandbox2/comms.h" #include "sandboxed_api/sandbox2/comms.h"
#include "sandboxed_api/sandbox2/executor.h" #include "sandboxed_api/sandbox2/executor.h"
#include "sandboxed_api/sandbox2/ipc.h" #include "sandboxed_api/sandbox2/ipc.h"
@ -60,7 +60,7 @@ class Monitor final {
void Run(); void Run();
// Getters for private fields. // Getters for private fields.
bool IsDone() const { return done_.load(std::memory_order_acquire); } bool IsDone() const { return done_notification_.HasBeenNotified(); }
// Getter/Setter for wait_for_execve_. // Getter/Setter for wait_for_execve_.
bool IsActivelyMonitoring(); bool IsActivelyMonitoring();
@ -156,15 +156,14 @@ class Monitor final {
// Parent (the Sandbox2 object) waits on it, until we either enable // Parent (the Sandbox2 object) waits on it, until we either enable
// monitoring of a process (sandboxee) successfully, or the setup process // monitoring of a process (sandboxee) successfully, or the setup process
// fails. // fails.
absl::BlockingCounter setup_counter_; absl::Notification setup_notification_;
// The field indicates whether the sandboxing task has been completed (either
// successfully or with error).
absl::Notification done_notification_;
// The main tracked PID. // The main tracked PID.
pid_t pid_ = -1; pid_t pid_ = -1;
// The field indicates whether the sandboxing task has been completed (either
// successfully or with error).
std::atomic<bool> done_{false};
absl::Mutex done_mutex_;
// False iff external kill is requested // False iff external kill is requested
std::atomic_flag external_kill_request_flag_; std::atomic_flag external_kill_request_flag_;
// False iff dump stack is requested // False iff dump stack is requested

View File

@ -21,7 +21,6 @@
#include <string> #include <string>
#include "absl/memory/memory.h" #include "absl/memory/memory.h"
#include "absl/synchronization/blocking_counter.h"
#include "sandboxed_api/sandbox2/monitor.h" #include "sandboxed_api/sandbox2/monitor.h"
#include "sandboxed_api/sandbox2/result.h" #include "sandboxed_api/sandbox2/result.h"
#include "sandboxed_api/util/canonical_errors.h" #include "sandboxed_api/util/canonical_errors.h"
@ -39,9 +38,8 @@ sapi::StatusOr<Result> Sandbox2::AwaitResultWithTimeout(
CHECK(monitor_ != nullptr) << "Sandbox was not launched yet"; CHECK(monitor_ != nullptr) << "Sandbox was not launched yet";
CHECK(monitor_thread_ != nullptr) << "Sandbox was already waited on"; CHECK(monitor_thread_ != nullptr) << "Sandbox was already waited on";
absl::MutexLock lock(&monitor_->done_mutex_); auto done =
auto done = monitor_->done_mutex_.AwaitWithTimeout( monitor_->done_notification_.WaitForNotificationWithTimeout(timeout);
absl::Condition(monitor_.get(), &Monitor::IsDone), timeout);
if (!done) { if (!done) {
return ::sapi::DeadlineExceededError( return ::sapi::DeadlineExceededError(
"Sandbox did not finish within timeout"); "Sandbox did not finish within timeout");
@ -124,7 +122,7 @@ void Sandbox2::Launch() {
// Wait for the Monitor to set-up the sandboxee correctly (or fail while // Wait for the Monitor to set-up the sandboxee correctly (or fail while
// doing that). From here on, it is safe to use the IPC object for // doing that). From here on, it is safe to use the IPC object for
// non-sandbox-related data exchange. // non-sandbox-related data exchange.
monitor_->setup_counter_.Wait(); monitor_->setup_notification_.WaitForNotification();
} }
} // namespace sandbox2 } // namespace sandbox2