From 1b65ebe2e0880e27ac3de4a8fdead6b0dd583b9e Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 8 Sep 2020 17:40:22 -0700 Subject: [PATCH] adb: fix `push --sync` with multiple inputs. Previously, when push --sync was used with multiple files, we tried to stat a remote file with pending acknowledgements still in the socket buffer, which leads to an abort when we read ID_OKAY instead of the response we were expecting. This patch changes the behavior to always wait for acknowledgements if we're pushing with --sync (or `adb sync`). This results in a ~100ms regression during a 10 second sync of every file on the system image, but that's not very much, so perhaps we should consider doing this always. Bug: http://b/166155032 Test: test_device.py Change-Id: I7110243071ba5fbaa77dcc76f42a19d9ed2b4195 --- adb/client/file_sync_client.cpp | 4 ++-- adb/test_device.py | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/adb/client/file_sync_client.cpp b/adb/client/file_sync_client.cpp index 8bbe2a878..2e8b97566 100644 --- a/adb/client/file_sync_client.cpp +++ b/adb/client/file_sync_client.cpp @@ -1053,7 +1053,7 @@ static bool sync_send(SyncConnection& sc, const std::string& lpath, const std::s if (!sc.SendSmallFile(rpath, mode, lpath, rpath, mtime, buf, data_length, dry_run)) { return false; } - return sc.ReadAcknowledgements(); + return sc.ReadAcknowledgements(sync); #endif } @@ -1077,7 +1077,7 @@ static bool sync_send(SyncConnection& sc, const std::string& lpath, const std::s return false; } } - return sc.ReadAcknowledgements(); + return sc.ReadAcknowledgements(sync); } static bool sync_recv_v1(SyncConnection& sc, const char* rpath, const char* lpath, const char* name, diff --git a/adb/test_device.py b/adb/test_device.py index c1caafcf6..a92d4a711 100755 --- a/adb/test_device.py +++ b/adb/test_device.py @@ -1271,6 +1271,31 @@ class FileOperationsTest: if temp_dir is not None: shutil.rmtree(temp_dir) + def test_push_sync_multiple(self): + """Sync multiple host directories to a specific path.""" + + try: + temp_dir = tempfile.mkdtemp() + temp_files = make_random_host_files(in_dir=temp_dir, num_files=32) + + device_dir = posixpath.join(self.DEVICE_TEMP_DIR, 'sync_src_dst') + + # Clean up any stale files on the device. + device = adb.get_device() # pylint: disable=no-member + device.shell(['rm', '-rf', device_dir]) + device.shell(['mkdir', '-p', device_dir]) + + host_paths = [os.path.join(temp_dir, x.base_name) for x in temp_files] + device.push(host_paths, device_dir, sync=True) + + self.verify_sync(device, temp_files, device_dir) + + self.device.shell(['rm', '-rf', self.DEVICE_TEMP_DIR]) + finally: + if temp_dir is not None: + shutil.rmtree(temp_dir) + + def test_push_dry_run_nonexistent_file(self): """Push with dry run."""