From 83d11da5ae37fc04247bdaf17e8305295fb186d3 Mon Sep 17 00:00:00 2001
From: Alexander Zaft <a.zaft@fz-juelich.de>
Date: Wed, 14 Jun 2023 09:57:29 +0200
Subject: [PATCH] server: Add signal handling

previously, no cleanup would be performed, SIGINTs being catched above
the server.run() function
+ signal handler for SIGINT, SIGTERM
* put serve_forever in its own thread, to be able to call shutdown() on
  the server

Change-Id: Ia5c021f3d6fd60ff2b9012c10e30715f93104977
Reviewed-on: https://forge.frm2.tum.de/review/c/secop/frappy/+/31340
Tested-by: Jenkins Automated Tests <pedersen+jenkins@frm2.tum.de>
Reviewed-by: Alexander Zaft <a.zaft@fz-juelich.de>
---
 bin/frappy-server | 16 +++++++---------
 frappy/server.py  | 42 ++++++++++++++++++++++++++++++------------
 2 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/bin/frappy-server b/bin/frappy-server
index 3d6993fb..d1c5cf94 100755
--- a/bin/frappy-server
+++ b/bin/frappy-server
@@ -1,6 +1,5 @@
 #!/usr/bin/env python3
 # pylint: disable=invalid-name
-#  -*- coding: utf-8 -*-
 # *****************************************************************************
 #
 # This program is free software; you can redistribute it and/or modify it under
@@ -23,8 +22,8 @@
 #
 # *****************************************************************************
 
-import sys
 import argparse
+import sys
 from os import path
 
 # Add import path for inplace usage
@@ -61,8 +60,9 @@ def parseArgv(argv):
                         action='store',
                         help="comma separated list of cfg files,\n"
                         "defaults to <name_of_the_instance>.\n"
-                        "cfgfiles given without '.cfg' extension are searched in the configuration directory, "
-                        "else they are treated as path names",
+                        "cfgfiles given without '.cfg' extension are searched"
+                        " in the configuration directory,"
+                        " else they are treated as path names",
                         default=None)
     parser.add_argument('-g',
                         '--gencfg',
@@ -96,15 +96,13 @@ def main(argv=None):
     generalConfig.init(args.gencfg)
     logger.init(loglevel)
 
-    srv = Server(args.name, logger.log, cfgfiles=args.cfgfiles, interface=args.port, testonly=args.test)
+    srv = Server(args.name, logger.log, cfgfiles=args.cfgfiles,
+                 interface=args.port, testonly=args.test)
 
     if args.daemonize:
         srv.start()
     else:
-        try:
-            srv.run()
-        except KeyboardInterrupt:
-            pass
+        srv.run()
 
 
 if __name__ == '__main__':
diff --git a/frappy/server.py b/frappy/server.py
index e46ed027..cf9f8365 100644
--- a/frappy/server.py
+++ b/frappy/server.py
@@ -1,4 +1,3 @@
-#  -*- coding: utf-8 -*-
 # *****************************************************************************
 #
 # This program is free software; you can redistribute it and/or modify it under
@@ -24,16 +23,17 @@
 """Define helpers"""
 
 import os
+import signal
 import sys
 import traceback
 from collections import OrderedDict
 
-from frappy.errors import ConfigError, SECoPError
-from frappy.lib import formatException, get_class, generalConfig
-from frappy.lib.multievent import MultiEvent
-from frappy.params import PREDEFINED_ACCESSIBLES
-from frappy.modules import Attached
 from frappy.config import load_config
+from frappy.errors import ConfigError, SECoPError
+from frappy.lib import formatException, generalConfig, get_class, mkthread
+from frappy.lib.multievent import MultiEvent
+from frappy.modules import Attached
+from frappy.params import PREDEFINED_ACCESSIBLES
 
 try:
     from daemon import DaemonContext
@@ -106,6 +106,12 @@ class Server:
 
         self._cfgfiles = cfgfiles
         self._pidfile = os.path.join(generalConfig.piddir, name + '.pid')
+        signal.signal(signal.SIGINT, self.signal_handler)
+        signal.signal(signal.SIGTERM, self.signal_handler)
+
+    def signal_handler(self, _num, _frame):
+        if hasattr(self, 'interface') and self.interface:
+            self.shutdown()
 
     def start(self):
         if not DaemonContext:
@@ -127,7 +133,7 @@ class Server:
         return f"{cls.__name__} class don't know how to handle option(s): {', '.join(options)}"
 
     def restart_hook(self):
-        pass
+        """Actions to be done on restart. May be overridden by a subclass."""
 
     def run(self):
         global systemd  # pylint: disable=global-statement
@@ -157,15 +163,27 @@ class Server:
                 self.log.info('startup done, handling transport messages')
                 if systemd:
                     systemd.daemon.notify("READY=1\nSTATUS=accepting requests")
-                self.interface.serve_forever()
-                self.interface.server_close()
+                t = mkthread(self.interface.serve_forever)
+                # we wait here on the thread finishing, which means we got a
+                # signal to shut down or an exception was raised
+                # TODO: get the exception (and re-raise?)
+                t.join()
+                self.interface = None  # fine due to the semantics of 'with'
+                # server_close() called by 'with'
+
+            self.log.info(f'stopped listenning, cleaning up'
+                          f' {len(self.modules)} modules')
+            # if systemd:
+            #     if self._restart:
+            #         systemd.daemon.notify('RELOADING=1')
+            #     else:
+            #         systemd.daemon.notify('STOPPING=1')
             for name in self._getSortedModules():
                 self.modules[name].shutdownModule()
             if self._restart:
                 self.restart_hook()
-                self.log.info('restart')
-            else:
-                self.log.info('shut down')
+                self.log.info('restarting')
+        self.log.info('shut down')
 
     def restart(self):
         if not self._restart: