Skip to content

Commit

Permalink
control/server.py: use signal TERM when stopping subprocesses
Browse files Browse the repository at this point in the history
- partial revert of 0370fe8

Signed-off-by: Alexander Indenbaum <[email protected]>
  • Loading branch information
Alexander Indenbaum authored and baum committed Aug 7, 2024
1 parent d05304b commit c76cf0e
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 9 deletions.
8 changes: 5 additions & 3 deletions control/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,10 @@ def __enter__(self):

def __exit__(self, exc_type, exc_value, traceback):
"""Cleans up SPDK and server instances."""
if exc_type is not None:
if exc_type is not None and exc_type is not SystemExit:
self.logger.exception("GatewayServer exception occurred:")
else:
self.logger.info("GatewayServer is terminating gracefully...")

gw_name = self.name
gw_logger = self.gw_logger_object
Expand Down Expand Up @@ -432,8 +434,8 @@ def _stop_subprocess(self, proc, timeout):
self.logger.error(f"{self.name} pid {proc.pid} "
f"already terminated, exit code: {return_code}")
else:
self.logger.info(f"Aborting ({self.name}) pid {proc.pid}...")
proc.send_signal(signal.SIGABRT)
self.logger.info(f"Terminating sub process of ({self.name}) pid {proc.pid} args {proc.args} ...")
proc.terminate()

try:
proc.communicate(timeout=timeout)
Expand Down
12 changes: 6 additions & 6 deletions tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ def validate_exception(self, e):
assert(pid > 0)
assert(code == 1)

def assert_core_files(self, directory_path):
def assert_no_core_files(self, directory_path):
assert(os.path.exists(directory_path) and os.path.isdir(directory_path))
files = [f for f in os.listdir(directory_path) if os.path.isfile(os.path.join(directory_path, f)) and f.startswith("core.")]
assert(len(files) > 0)
assert(len(files) == 0)

def test_spdk_exception(self):
"""Tests spdk sub process exiting with error."""
Expand All @@ -38,15 +38,15 @@ def test_spdk_exception(self):
gateway.serve()
self.validate_exception(cm.exception)

def test_spdk_abort(self):
"""Tests spdk sub process dumps core on during normal shutdown."""
def test_no_coredumps_on_gracefull_shutdown(self):
"""Tests gateway's sub processes do not dump cores on gracefull shutdown."""
with GatewayServer(copy.deepcopy(self.config)) as gateway:
gateway.set_group_id(0)
gateway.serve()
time.sleep(10)
# exited context, spdk process should be aborted here by __exit__()
# exited context, sub processes should terminate gracefully
time.sleep(10) # let it dump
self.assert_core_files("/tmp/coredump")
self.assert_no_core_files("/tmp/coredump")

def test_spdk_multi_gateway_exception(self):
"""Tests spdk sub process exiting with error, in multi gateway configuration."""
Expand Down

0 comments on commit c76cf0e

Please sign in to comment.