Modernize the transaction API

PiperOrigin-RevId: 295712938
Change-Id: Iaf4c9668bb0b48555679fef822fe424277540d1f

sandboxed_api/examples/sum/main_sum.cc

@@ -58,7 +58,7 @@ class SumTransaction : public sapi::Transaction {
 };
 
 sapi::Status SumTransaction::Main() {
-  SumApi f(GetSandbox());
+  SumApi f(sandbox());
   SAPI_ASSIGN_OR_RETURN(int v, f.sum(1000, 337));
   LOG(INFO) << "1000 + 337 = " << v;
   TRANSACTION_FAIL_IF_NOT(v == 1337, "1000 + 337 != 1337");
@@ -89,10 +89,10 @@ sapi::Status SumTransaction::Main() {
   // Gets symbol address and prints its value.
   int* ssaddr;
   SAPI_RETURN_IF_ERROR(
-      GetSandbox()->Symbol("sumsymbol", reinterpret_cast<void**>(&ssaddr)));
+      sandbox()->Symbol("sumsymbol", reinterpret_cast<void**>(&ssaddr)));
   sapi::v::Int sumsymbol;
   sumsymbol.SetRemote(ssaddr);
-  SAPI_RETURN_IF_ERROR(GetSandbox()->TransferFromSandboxee(&sumsymbol));
+  SAPI_RETURN_IF_ERROR(sandbox()->TransferFromSandboxee(&sumsymbol));
   LOG(INFO) << "sumsymbol value (exp: 5): " << sumsymbol.GetValue()
             << ", address: " << ssaddr;
   TRANSACTION_FAIL_IF_NOT(sumsymbol.GetValue() == 5,
@@ -120,7 +120,7 @@ sapi::Status SumTransaction::Main() {
   LOG(INFO) << "Print: '" << hwstr << "' via puts()";
   sapi::v::Array<const char> hwarr(hwstr, sizeof(hwstr));
   sapi::v::Int ret;
-  SAPI_RETURN_IF_ERROR(GetSandbox()->Call("puts", &ret, hwarr.PtrBefore()));
+  SAPI_RETURN_IF_ERROR(sandbox()->Call("puts", &ret, hwarr.PtrBefore()));
   TRANSACTION_FAIL_IF_NOT(ret.GetValue() == 15, "puts('Hello World!!!') != 15");
 
   sapi::v::Int vp;
@@ -144,13 +144,13 @@ sapi::Status SumTransaction::Main() {
   // Fd transfer test.
   int fdesc = open("/proc/self/exe", O_RDONLY);
   sapi::v::Fd fd(fdesc);
-  SAPI_RETURN_IF_ERROR(GetSandbox()->TransferToSandboxee(&fd));
+  SAPI_RETURN_IF_ERROR(sandbox()->TransferToSandboxee(&fd));
   LOG(INFO) << "remote_fd = " << fd.GetRemoteFd();
   TRANSACTION_FAIL_IF_NOT(fd.GetRemoteFd() == 3, "remote_fd != 3");
 
   fdesc = open("/proc/self/comm", O_RDONLY);
   sapi::v::Fd fd2(fdesc);
-  SAPI_RETURN_IF_ERROR(GetSandbox()->TransferToSandboxee(&fd2));
+  SAPI_RETURN_IF_ERROR(sandbox()->TransferToSandboxee(&fd2));
   LOG(INFO) << "remote_fd2 = " << fd2.GetRemoteFd();
   TRANSACTION_FAIL_IF_NOT(fd2.GetRemoteFd() == 4, "remote_fd2 != 4");
 
@@ -158,19 +158,19 @@ sapi::Status SumTransaction::Main() {
   char buffer[1024] = {0};
   sapi::v::Array<char> buf(buffer, sizeof(buffer));
   sapi::v::UInt size(128);
-  SAPI_RETURN_IF_ERROR(GetSandbox()->Call("read", &ret, &fd2, buf.PtrBoth(), &size));
+  SAPI_RETURN_IF_ERROR(sandbox()->Call("read", &ret, &fd2, buf.PtrBoth(), &size));
   LOG(INFO) << "Read from /proc/self/comm = [" << buffer << "]";
 
   // Close test.
-  SAPI_RETURN_IF_ERROR(fd2.CloseRemoteFd(GetSandbox()->GetRpcChannel()));
+  SAPI_RETURN_IF_ERROR(fd2.CloseRemoteFd(sandbox()->GetRpcChannel()));
   memset(buffer, 0, sizeof(buffer));
-  SAPI_RETURN_IF_ERROR(GetSandbox()->Call("read", &ret, &fd2, buf.PtrBoth(), &size));
+  SAPI_RETURN_IF_ERROR(sandbox()->Call("read", &ret, &fd2, buf.PtrBoth(), &size));
   LOG(INFO) << "Read from closed /proc/self/comm = [" << buffer << "]";
 
   // Pass fd as function arg example.
   fdesc = open("/proc/self/statm", O_RDONLY);
   sapi::v::Fd fd3(fdesc);
-  SAPI_RETURN_IF_ERROR(GetSandbox()->TransferToSandboxee(&fd3));
+  SAPI_RETURN_IF_ERROR(sandbox()->TransferToSandboxee(&fd3));
   SAPI_ASSIGN_OR_RETURN(int r2, f.read_int(fd3.GetRemoteFd()));
   LOG(INFO) << "statm value (should not be 0) = " << r2;
 

sandboxed_api/sapi_test.cc

@@ -72,10 +72,10 @@ void BenchmarkSandboxRestartOverhead(benchmark::State& state) {
 BENCHMARK(BenchmarkSandboxRestartOverhead);
 
 void BenchmarkSandboxRestartForkserverOverhead(benchmark::State& state) {
-  sapi::BasicTransaction st{absl::make_unique<StringopSandbox>()};
+  sapi::BasicTransaction st(absl::make_unique<StringopSandbox>());
   for (auto _ : state) {
     EXPECT_THAT(st.Run(InvokeNop), IsOk());
-    EXPECT_THAT(st.GetSandbox()->Restart(true), IsOk());
+    EXPECT_THAT(st.sandbox()->Restart(true), IsOk());
   }
 }
 BENCHMARK(BenchmarkSandboxRestartForkserverOverhead);
@@ -84,7 +84,7 @@ void BenchmarkSandboxRestartForkserverOverheadForced(benchmark::State& state) {
   sapi::BasicTransaction st{absl::make_unique<StringopSandbox>()};
   for (auto _ : state) {
     EXPECT_THAT(st.Run(InvokeNop), IsOk());
-    EXPECT_THAT(st.GetSandbox()->Restart(false), IsOk());
+    EXPECT_THAT(st.sandbox()->Restart(false), IsOk());
   }
 }
 BENCHMARK(BenchmarkSandboxRestartForkserverOverheadForced);

sandboxed_api/transaction.cc

@@ -23,23 +23,23 @@ constexpr absl::Duration TransactionBase::kDefaultTimeLimit;
 sapi::Status TransactionBase::RunTransactionFunctionInSandbox(
     const std::function<sapi::Status()>& f) {
   // Run Main(), invoking Init() if this hasn't been yet done.
-  SAPI_RETURN_IF_ERROR(GetSandbox()->Init());
+  SAPI_RETURN_IF_ERROR(sandbox_->Init());
 
   // Set the wall-time limit for this transaction run, and clean it up
   // afterwards, no matter what the result.
-  SAPI_RETURN_IF_ERROR(GetSandbox()->SetWallTimeLimit(GetTimeLimit()));
+  SAPI_RETURN_IF_ERROR(sandbox_->SetWallTimeLimit(GetTimeLimit()));
   struct TimeCleanup {
     ~TimeCleanup() {
-      if (capture->GetSandbox()->IsActive()) {
+      if (capture->sandbox_->IsActive()) {
-        capture->GetSandbox()->SetWallTimeLimit(0).IgnoreError();
+        capture->sandbox_->SetWallTimeLimit(0).IgnoreError();
       }
     }
     TransactionBase* capture;
-  } sandbox_cleanup{this};
+  } sandbox_cleanup = {this};
 
-  if (!GetInited()) {
+  if (!initialized_) {
     SAPI_RETURN_IF_ERROR(Init());
-    SetInited(true);
+    initialized_ = true;
   }
 
   return f();
@@ -50,24 +50,24 @@ sapi::Status TransactionBase::RunTransactionLoop(
   // Try to run Main() for a few times, return error if none of the tries
   // succeeded.
   sapi::Status status;
-  for (int i = 0; i <= GetRetryCnt(); i++) {
+  for (int i = 0; i <= retry_count_; ++i) {
     status = RunTransactionFunctionInSandbox(f);
     if (status.ok()) {
       return status;
     }
-    GetSandbox()->Terminate();
+    sandbox_->Terminate();
-    SetInited(false);
+    initialized_ = false;
   }
 
-  LOG(ERROR) << "Tried " << (GetRetryCnt() + 1) << " times to run the "
+  LOG(ERROR) << "Tried " << (retry_count_ + 1) << " times to run the "
              << "transaction, but it failed. SAPI error: '" << status
              << "'. Latest sandbox error: '"
-             << GetSandbox()->AwaitResult().ToString() << "'";
+             << sandbox_->AwaitResult().ToString() << "'";
   return status;
 }
 
 TransactionBase::~TransactionBase() {
-  if (GetInited()) {
+  if (initialized_) {
     Finish().IgnoreError();
   }
 }

sandboxed_api/transaction.h

@@ -51,13 +51,14 @@ class TransactionBase {
  public:
   TransactionBase(const TransactionBase&) = delete;
   TransactionBase& operator=(const TransactionBase&) = delete;
+
   virtual ~TransactionBase();
 
-  // Getter/Setter for retry_cnt_.
+  // Getter/Setter for retry_count_.
-  int GetRetryCnt() const { return retry_cnt_; }
+  int retry_count() const { return retry_count_; }
-  void SetRetryCnt(int retry_count) {
+  void set_retry_count(int value) {
-    CHECK_GE(retry_count, 0);
+    CHECK_GE(value, 0);
-    retry_cnt_ = retry_count;
+    retry_count_ = value;
   }
 
   // Getter/Setter for time_limit_.
@@ -67,30 +68,28 @@ class TransactionBase {
     time_limit_ = absl::ToTimeT(absl::UnixEpoch() + time_limit);
   }
 
-  // Getter/Setter for inited_.
+  bool IsInitialized() const { return initialized_; }
-  bool GetInited() const { return inited_; }
-  void SetInited(bool inited) { inited_ = inited; }
 
   // Getter for the sandbox_.
-  Sandbox* GetSandbox() { return sandbox_.get(); }
+  Sandbox* sandbox() const { return sandbox_.get(); }
 
   // Restarts the sandbox.
   // WARNING: This will invalidate any references to the remote process, make
-  // sure you don't keep any var's or FD's to the remote process when calling
+  //          sure you don't keep any vars or FDs to the remote process when
-  // this.
+  //          calling this.
   sapi::Status Restart() {
-    if (inited_) {
+    if (initialized_) {
       Finish().IgnoreError();
-      inited_ = false;
+      initialized_ = false;
     }
     return sandbox_->Restart(true);
   }
 
  protected:
   explicit TransactionBase(std::unique_ptr<Sandbox> sandbox)
-      : retry_cnt_(kDefaultRetryCnt),
+      : retry_count_(kDefaultRetryCount),
         time_limit_(absl::ToTimeT(absl::UnixEpoch() + kDefaultTimeLimit)),
-        inited_(false),
+        initialized_(false),
         sandbox_(std::move(sandbox)) {}
 
   // Runs the main (retrying) transaction loop.
@@ -98,7 +97,7 @@ class TransactionBase {
 
  private:
   // Number of default transaction execution re-tries, in case of failures.
-  static constexpr int kDefaultRetryCnt = 1;
+  static constexpr int kDefaultRetryCount = 1;
 
   // Wall-time limit for a single transaction execution (60 s.).
   static constexpr absl::Duration kDefaultTimeLimit = absl::Seconds(60);
@@ -108,7 +107,7 @@ class TransactionBase {
   sapi::Status RunTransactionFunctionInSandbox(
       const std::function<sapi::Status()>& f);
 
-  // Initialization routine of the sandboxed process that ill be called only
+  // Initialization routine of the sandboxed process that will be called only
   // once upon sandboxee startup.
   virtual sapi::Status Init() { return sapi::OkStatus(); }
 
@@ -117,14 +116,14 @@ class TransactionBase {
   virtual sapi::Status Finish() { return sapi::OkStatus(); }
 
   // Number of tries this transaction will be re-executed until it succeeds.
-  int retry_cnt_;
+  int retry_count_;
 
   // Time (wall-time) limit for a single Run() call (in seconds). 0 means: no
   // wall-time limit.
   time_t time_limit_;
 
   // Has Init() finished with success?
-  bool inited_;
+  bool initialized_;
 
   // The main sapi::Sandbox object.
   std::unique_ptr<Sandbox> sandbox_;
@@ -172,13 +171,13 @@ class BasicTransaction final : public TransactionBase {
         init_function_(static_cast<InitFunction>(init_function)),
         finish_function_(static_cast<FinishFunction>(fini_function)) {}
 
-  // Run any function as body of the transaction that matches our expectations (
+  // Run any function as body of the transaction that matches our expectations
-  // that is: Returning a sapi::Status and accepting a Sandbox object as first
+  // (that is: Returning a Status and accepting a Sandbox object as first
   // parameter).
   template <typename T, typename... Args>
   sapi::Status Run(T func, Args&&... args) {
     return RunTransactionLoop(
-        [&] { return func(GetSandbox(), std::forward<Args>(args)...); });
+        [&] { return func(sandbox(), std::forward<Args>(args)...); });
   }
 
  private:
@@ -186,11 +185,11 @@ class BasicTransaction final : public TransactionBase {
   FinishFunction finish_function_;
 
   sapi::Status Init() final {
-    return init_function_ ? init_function_(GetSandbox()) : sapi::OkStatus();
+    return init_function_ ? init_function_(sandbox()) : sapi::OkStatus();
   }
 
   sapi::Status Finish() final {
-    return finish_function_ ? finish_function_(GetSandbox()) : sapi::OkStatus();
+    return finish_function_ ? finish_function_(sandbox()) : sapi::OkStatus();
   }
 };