mirror of
https://gerrit.googlesource.com/git-repo
synced 2026-05-30 14:39:46 +00:00
sync: Re-raise KeyboardInterrupt in main process
When running sync -j1, worker functions run directly in the main process. Swallowing KeyboardInterrupt causes the loop to continue to the next project instead of aborting. Re-raise KeyboardInterrupt if running in the MainProcess, while maintaining the suppression of stack traces in worker processes. Bug: 468170157 Change-Id: I156d66bc209a265f7fa25eea0eb88737d1b51a34 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/581342 Commit-Queue: Gavin Mak <gavinmak@google.com> Reviewed-by: Mike Frysinger <vapier@google.com> Tested-by: Gavin Mak <gavinmak@google.com>
This commit is contained in:
committed by
gerrit-scoped@luci-project-accounts.iam.gserviceaccount.com
parent
7cc99b24c1
commit
d5037230e9
@@ -101,6 +101,11 @@ class Command:
|
||||
def WantPager(self, _opt):
|
||||
return False
|
||||
|
||||
@staticmethod
|
||||
def is_multiprocessing_active() -> bool:
|
||||
"""Whether the current process is a worker in a pool."""
|
||||
return multiprocessing.current_process().name != "MainProcess"
|
||||
|
||||
def ReadEnvironmentOptions(self, opts):
|
||||
"""Set options from environment variables."""
|
||||
|
||||
|
||||
@@ -841,6 +841,8 @@ later is required to fix a server side protocol bug.
|
||||
)
|
||||
except KeyboardInterrupt:
|
||||
logger.error("Keyboard interrupt while processing %s", project.name)
|
||||
if not cls.is_multiprocessing_active():
|
||||
raise
|
||||
except GitError as e:
|
||||
logger.error("error.GitError: Cannot fetch %s", e)
|
||||
errors.append(e)
|
||||
@@ -1104,6 +1106,8 @@ later is required to fix a server side protocol bug.
|
||||
errors.extend(syncbuf.errors)
|
||||
except KeyboardInterrupt:
|
||||
logger.error("Keyboard interrupt while processing %s", project.name)
|
||||
if not cls.is_multiprocessing_active():
|
||||
raise
|
||||
except GitError as e:
|
||||
logger.error(
|
||||
"error.GitError: Cannot checkout %s: %s", project.name, e
|
||||
@@ -2410,6 +2414,8 @@ later is required to fix a server side protocol bug.
|
||||
logger.error(
|
||||
"Keyboard interrupt while processing %s", project.name
|
||||
)
|
||||
if not cls.is_multiprocessing_active():
|
||||
raise
|
||||
except GitError as e:
|
||||
fetch_errors.append(e)
|
||||
logger.error("error.GitError: Cannot fetch %s", e)
|
||||
@@ -2460,6 +2466,8 @@ later is required to fix a server side protocol bug.
|
||||
logger.error(
|
||||
"Keyboard interrupt while processing %s", project.name
|
||||
)
|
||||
if not cls.is_multiprocessing_active():
|
||||
raise
|
||||
except GitError as e:
|
||||
checkout_errors.append(e)
|
||||
logger.error(
|
||||
|
||||
@@ -477,6 +477,56 @@ class GetPreciousObjectsState(unittest.TestCase):
|
||||
)
|
||||
|
||||
|
||||
class KeyboardInterruptTest(unittest.TestCase):
|
||||
"""Tests for KeyboardInterrupt handling in Sync operations."""
|
||||
|
||||
def setUp(self):
|
||||
self.project = mock.MagicMock(name="project")
|
||||
self.project.name = "project"
|
||||
self.project.relpath = "proj"
|
||||
self.project.manifest.IsArchive = False
|
||||
self.opt = mock.Mock()
|
||||
self.opt.quiet = True
|
||||
self.opt.verbose = False
|
||||
self.opt.tags = False
|
||||
|
||||
self.sync_dict = {}
|
||||
|
||||
self.get_parallel_context_mock = {
|
||||
"projects": [self.project],
|
||||
"sync_dict": self.sync_dict,
|
||||
"ssh_proxy": None,
|
||||
}
|
||||
|
||||
@mock.patch("subcmds.sync.Sync.is_multiprocessing_active")
|
||||
def test_fetch_one_keyboard_interrupt_main_process(self, mock_is_active):
|
||||
"""Test that _FetchOne re-raises KeyboardInterrupt if not worker."""
|
||||
mock_is_active.return_value = False
|
||||
self.project.Sync_NetworkHalf.side_effect = KeyboardInterrupt()
|
||||
|
||||
with mock.patch.object(
|
||||
sync.Sync,
|
||||
"get_parallel_context",
|
||||
return_value=self.get_parallel_context_mock,
|
||||
):
|
||||
with self.assertRaises(KeyboardInterrupt):
|
||||
sync.Sync._FetchOne(self.opt, 0)
|
||||
|
||||
@mock.patch("subcmds.sync.Sync.is_multiprocessing_active")
|
||||
def test_fetch_one_keyboard_interrupt_worker_process(self, mock_is_active):
|
||||
"""Test that _FetchOne suppresses KeyboardInterrupt in workers."""
|
||||
mock_is_active.return_value = True
|
||||
self.project.Sync_NetworkHalf.side_effect = KeyboardInterrupt()
|
||||
|
||||
with mock.patch.object(
|
||||
sync.Sync,
|
||||
"get_parallel_context",
|
||||
return_value=self.get_parallel_context_mock,
|
||||
):
|
||||
result = sync.Sync._FetchOne(self.opt, 0)
|
||||
self.assertFalse(result.success)
|
||||
|
||||
|
||||
class CheckForBloatedProjects(unittest.TestCase):
|
||||
"""Tests for Sync._CheckForBloatedProjects."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user