Fix a race when terminating sandbox from another thread

PiperOrigin-RevId: 436695251
Change-Id: I50599cefb346813f594982641c78dc902e10ccb5
This commit is contained in:
Wiktor Garbacz 2022-03-23 03:34:56 -07:00 committed by Copybara-Service
parent ab9c4afb15
commit fb690062cf
3 changed files with 24 additions and 2 deletions

View File

@ -200,6 +200,7 @@ absl::Status Sandbox::Init() {
s2_ = absl::make_unique<sandbox2::Sandbox2>(std::move(executor),
std::move(s2p), CreateNotifier());
s2_awaited_ = false;
auto res = s2_->RunAsync();
comms_ = s2_->comms();
@ -432,9 +433,9 @@ absl::StatusOr<std::string> Sandbox::GetCString(const v::RemotePtr& str,
}
const sandbox2::Result& Sandbox::AwaitResult() {
if (s2_) {
if (s2_ && !s2_awaited_) {
result_ = s2_->AwaitResult();
s2_.reset(nullptr);
s2_awaited_ = true;
}
return result_;
}

View File

@ -149,6 +149,10 @@ class Sandbox {
// The main sandbox2::Sandbox2 object.
std::unique_ptr<sandbox2::Sandbox2> s2_;
// Marks whether Sandbox2 result was already fetched.
// We cannot just delete s2_ as Terminate might be called from another thread
// and comms object can be still in use then.
bool s2_awaited_ = false;
// Result of the most recent sandbox execution
sandbox2::Result result_;

View File

@ -14,6 +14,8 @@
#include <fcntl.h>
#include <thread> // NOLINT(build/c++11)
#include "benchmark/benchmark.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@ -262,5 +264,20 @@ TEST(SandboxTest, NoRaceInAwaitResult) {
EXPECT_THAT(result.final_status(), Eq(sandbox2::Result::VIOLATION));
}
TEST(SandboxTest, NoRaceInConcurrentTerminate) {
SumSandbox sandbox;
ASSERT_THAT(sandbox.Init(), IsOk());
SumApi api(&sandbox);
std::thread th([&sandbox] {
// Sleep so that the call already starts
absl::SleepFor(absl::Seconds(1));
sandbox.Terminate(/*attempt_graceful_exit=*/false);
});
EXPECT_THAT(api.sleep_for_sec(10), StatusIs(absl::StatusCode::kUnavailable));
th.join();
const auto& result = sandbox.AwaitResult();
EXPECT_THAT(result.final_status(), Eq(sandbox2::Result::EXTERNAL_KILL));
}
} // namespace
} // namespace sapi