Merge "vts_libsnapshot_test: Fix test flakiness." am: 0dfbdc5bdb

Original change: https://android-review.googlesource.com/c/platform/system/core/+/2199352

Change-Id: I83eb0010991c945233bf9dbff56e5322736d8311
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

fs_mgr/libsnapshot/snapshot.cpp

@@ -1498,7 +1498,6 @@ void SnapshotManager::AcknowledgeMergeSuccess(LockedFile* lock) {
     if (UpdateUsesUserSnapshots(lock) && !device()->IsTestDevice()) {
         if (snapuserd_client_) {
             snapuserd_client_->DetachSnapuserd();
-            snapuserd_client_->CloseConnection();
             snapuserd_client_ = nullptr;
         }
     }
@@ -2794,7 +2793,6 @@ bool SnapshotManager::UnmapAllSnapshots(LockedFile* lock) {
     if (snapuserd_client_) {
         LOG(INFO) << "Shutdown snapuserd daemon";
         snapuserd_client_->DetachSnapuserd();
-        snapuserd_client_->CloseConnection();
         snapuserd_client_ = nullptr;
     }
 
@@ -3317,7 +3315,7 @@ Return SnapshotManager::CreateUpdateSnapshots(const DeltaArchiveManifest& manife
         }
         if (snapuserd_client) {
             snapuserd_client->DetachSnapuserd();
-            snapuserd_client->CloseConnection();
+            snapuserd_client = nullptr;
         }
     }
 

fs_mgr/libsnapshot/snapshot_test.cpp

@@ -107,6 +107,12 @@ class SnapshotTest : public ::testing::Test {
 
   protected:
     void SetUp() override {
+        const testing::TestInfo* const test_info =
+                testing::UnitTest::GetInstance()->current_test_info();
+        test_name_ = test_info->test_suite_name() + "/"s + test_info->name();
+
+        LOG(INFO) << "Starting test: " << test_name_;
+
         SKIP_IF_NON_VIRTUAL_AB();
 
         SetupProperties();
@@ -152,10 +158,14 @@ class SnapshotTest : public ::testing::Test {
     void TearDown() override {
         RETURN_IF_NON_VIRTUAL_AB();
 
+        LOG(INFO) << "Tearing down SnapshotTest test: " << test_name_;
+
         lock_ = nullptr;
 
         CleanupTestArtifacts();
         SnapshotTestPropertyFetcher::TearDown();
+
+        LOG(INFO) << "Teardown complete for test: " << test_name_;
     }
 
     void InitializeState() {
@@ -487,6 +497,7 @@ class SnapshotTest : public ::testing::Test {
     android::fiemap::IImageManager* image_manager_ = nullptr;
     std::string fake_super_;
     bool snapuserd_required_ = false;
+    std::string test_name_;
 };
 
 TEST_F(SnapshotTest, CreateSnapshot) {
@@ -1003,6 +1014,8 @@ class SnapshotUpdateTest : public SnapshotTest {
     void TearDown() override {
         RETURN_IF_NON_VIRTUAL_AB();
 
+        LOG(INFO) << "Tearing down SnapshotUpdateTest test: " << test_name_;
+
         Cleanup();
         SnapshotTest::TearDown();
     }
@@ -2797,7 +2810,6 @@ void KillSnapuserd() {
         return;
     }
     snapuserd_client->DetachSnapuserd();
-    snapuserd_client->CloseConnection();
 }
 
 }  // namespace snapshot

fs_mgr/libsnapshot/snapuserd/include/snapuserd/snapuserd_client.h

@@ -71,8 +71,6 @@ class SnapuserdClient {
     // must ONLY be called if the control device has already been deleted.
     bool WaitForDeviceDelete(const std::string& control_device);
 
-    void CloseConnection() { sockfd_ = {}; }
-
     // Detach snapuserd. This shuts down the listener socket, and will cause
     // snapuserd to gracefully exit once all handler threads have terminated.
     // This should only be used on first-stage instances of snapuserd.

fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.cpp

@@ -462,15 +462,14 @@ void UserSnapshotServer::AcceptClient() {
 }
 
 bool UserSnapshotServer::HandleClient(android::base::borrowed_fd fd, int revents) {
-    if (revents & POLLHUP) {
-        LOG(DEBUG) << "Snapuserd client disconnected";
-        return false;
-    }
-
     std::string str;
     if (!Recv(fd, &str)) {
         return false;
     }
+    if (str.empty() && (revents & POLLHUP)) {
+        LOG(DEBUG) << "Snapuserd client disconnected";
+        return false;
+    }
     if (!Receivemsg(fd, str)) {
         LOG(ERROR) << "Encountered error handling client message, revents: " << revents;
         return false;
@@ -650,6 +649,12 @@ void UserSnapshotServer::MonitorMerge() {
             while (active_merge_threads_ < kMaxMergeThreads && merge_handlers_.size() > 0) {
                 auto handler = merge_handlers_.front();
                 merge_handlers_.pop();
+
+                if (!handler->snapuserd()) {
+                    LOG(INFO) << "MonitorMerge: skipping deleted handler: " << handler->misc_name();
+                    continue;
+                }
+
                 LOG(INFO) << "Starting merge for partition: "
                           << handler->snapuserd()->GetMiscName();
                 handler->snapuserd()->InitiateMerge();