From f357312584a5d0de10f1d9c5ce515f64a0edba89 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Wed, 22 Feb 2023 10:13:18 +0100 Subject: [PATCH] [utils] term signal cleanup handlers add functions to register/unregister termination cleanup handlers --- client/X11/xf_cliprdr.c | 41 ++++++-- include/freerdp/utils/signal.h | 38 ++++++- libfreerdp/core/freerdp.c | 13 +++ libfreerdp/utils/signal.c | 178 +++++++++++++++++++++++++++++---- 4 files changed, 235 insertions(+), 35 deletions(-) diff --git a/client/X11/xf_cliprdr.c b/client/X11/xf_cliprdr.c index 107eed87d..803545e3d 100644 --- a/client/X11/xf_cliprdr.c +++ b/client/X11/xf_cliprdr.c @@ -53,6 +53,7 @@ #include #include +#include #include #include #include @@ -3067,6 +3068,29 @@ static struct fuse_lowlevel_ops xf_cliprdr_fuse_oper = { .opendir = xf_cliprdr_fuse_opendir, }; +static void fuse_session_terminate(xfClipboard* clipboard) +{ + if (!clipboard) + return; + + if (clipboard->fuse_sess) + fuse_session_exit(clipboard->fuse_sess); + + /* not elegant but works for umounting FUSE + fuse_chan must receieve a oper buf to unblock fuse_session_receive_buf function. + */ + WINPR_ASSERT(clipboard->delegate); + winpr_PathFileExists(clipboard->delegate->basePath); +} + +static void fuse_abort(int sig, const char* signame, void* context) +{ + xfClipboard* clipboard = (xfClipboard*)context; + + WLog_INFO(TAG, "signal %s [%d] aborting session", signame, sig); + fuse_session_terminate(clipboard); +} + static DWORD WINAPI xf_cliprdr_fuse_thread(LPVOID arg) { xfClipboard* clipboard = (xfClipboard*)arg; @@ -3108,11 +3132,13 @@ static DWORD WINAPI xf_cliprdr_fuse_thread(LPVOID arg) if ((clipboard->fuse_sess = fuse_session_new( &args, &xf_cliprdr_fuse_oper, sizeof(xf_cliprdr_fuse_oper), (void*)clipboard)) != NULL) { + freerdp_add_signal_cleanup_handler(clipboard, fuse_abort); if (0 == fuse_session_mount(clipboard->fuse_sess, clipboard->delegate->basePath)) { fuse_session_loop(clipboard->fuse_sess); fuse_session_unmount(clipboard->fuse_sess); } + freerdp_del_signal_cleanup_handler(clipboard, fuse_abort); fuse_session_destroy(clipboard->fuse_sess); } #else @@ -3123,11 +3149,13 @@ static DWORD WINAPI xf_cliprdr_fuse_thread(LPVOID arg) sizeof(xf_cliprdr_fuse_oper), (void*)clipboard); if (clipboard->fuse_sess != NULL) { + freerdp_add_signal_cleanup_handler(clipboard, fuse_abort); fuse_session_add_chan(clipboard->fuse_sess, ch); const int err = fuse_session_loop(clipboard->fuse_sess); if (err != 0) WLog_WARN(TAG, "fuse_session_loop failed with %d", err); fuse_session_remove_chan(ch); + freerdp_del_signal_cleanup_handler(clipboard, fuse_abort); fuse_session_destroy(clipboard->fuse_sess); } fuse_unmount(clipboard->delegate->basePath, ch); @@ -3145,7 +3173,7 @@ static DWORD WINAPI xf_cliprdr_fuse_thread(LPVOID arg) xfClipboard* xf_clipboard_new(xfContext* xfc, BOOL relieveFilenameRestriction) { - int i, n = 0; + int n = 0; rdpChannels* channels; xfClipboard* clipboard; const char* selectionAtom; @@ -3374,7 +3402,7 @@ void xf_clipboard_free(xfClipboard* clipboard) if (clipboard->numClientFormats) { - for (int i = 0; i < clipboard->numClientFormats; i++) + for (UINT32 i = 0; i < clipboard->numClientFormats; i++) { xfCliprdrFormat* format = &clipboard->clientFormats[i]; free(format->formatName); @@ -3384,14 +3412,7 @@ void xf_clipboard_free(xfClipboard* clipboard) #if defined(WITH_FUSE2) || defined(WITH_FUSE3) if (clipboard->fuse_thread) { - if (clipboard->fuse_sess) - fuse_session_exit(clipboard->fuse_sess); - - /* not elegant but works for umounting FUSE - fuse_chan must receieve a oper buf to unblock fuse_session_receive_buf function. - */ - winpr_PathFileExists(clipboard->delegate->basePath); - + fuse_session_terminate(clipboard); WaitForSingleObject(clipboard->fuse_thread, INFINITE); CloseHandle(clipboard->fuse_thread); } diff --git a/include/freerdp/utils/signal.h b/include/freerdp/utils/signal.h index 50847f405..7b95f7533 100644 --- a/include/freerdp/utils/signal.h +++ b/include/freerdp/utils/signal.h @@ -26,11 +26,6 @@ #ifndef _WIN32 #include #include - -FREERDP_API extern volatile sig_atomic_t terminal_needs_reset; -FREERDP_API extern int terminal_fildes; -FREERDP_API extern struct termios orig_flags; -FREERDP_API extern struct termios new_flags; #endif #ifdef __cplusplus @@ -38,8 +33,41 @@ extern "C" { #endif + typedef void (*freerdp_signal_handler_t)(int signum, const char* signame, void* context); + +#ifndef _WIN32 + FREERDP_API extern volatile sig_atomic_t terminal_needs_reset; + FREERDP_API extern int terminal_fildes; + FREERDP_API extern struct termios orig_flags; + FREERDP_API extern struct termios new_flags; +#endif + FREERDP_API int freerdp_handle_signals(void); + /** \brief registers a cleanup handler for non fatal signals. + * + * This allows cleaning up resources like with \b atexit but for signals. + * + * \param context a context for the clenaup handler. + * \param handler the function to call on cleanup. Must not be \b NULL + * + * \return \b TRUE if registered successfully, \b FALSE otherwise. + */ + FREERDP_API BOOL freerdp_add_signal_cleanup_handler(void* context, + freerdp_signal_handler_t handler); + + /** \brief unregisters a cleanup handler for non fatal signals. + * + * This allows removal of a cleanup handler for signals. + * + * \param context a context for the clenaup handler. + * \param handler the function to call on cleanup. Must not be \b NULL + * + * \return \b TRUE if unregistered successfully, \b FALSE otherwise. + */ + FREERDP_API BOOL freerdp_del_signal_cleanup_handler(void* context, + freerdp_signal_handler_t handler); + #ifdef __cplusplus } #endif diff --git a/libfreerdp/core/freerdp.c b/libfreerdp/core/freerdp.c index 996c20cc4..fab6ff02d 100644 --- a/libfreerdp/core/freerdp.c +++ b/libfreerdp/core/freerdp.c @@ -51,12 +51,22 @@ #include #include #include +#include #include "settings.h" #include "utils.h" #define TAG FREERDP_TAG("core") +static void sig_abort_connect(int signum, const char* signame, void* ctx) +{ + rdpContext* context = (rdpContext*)ctx; + + WLog_INFO(TAG, "Signal %s [%d], terminating session %p", signame, signum, context); + if (context) + freerdp_abort_connect_context(context); +} + /** Creates a new connection based on the settings found in the "instance" parameter * It will use the callbacks registered on the structure to process the pre/post connect operations * that the caller requires. @@ -100,6 +110,8 @@ static int freerdp_connect_begin(freerdp* instance) if (!freerdp_settings_set_default_order_support(settings)) return -1; + freerdp_add_signal_cleanup_handler(instance->context, sig_abort_connect); + IFCALLRET(instance->PreConnect, status, instance); instance->ConnectionCallbackState = CLIENT_STATE_PRECONNECT_PASSED; @@ -550,6 +562,7 @@ BOOL freerdp_disconnect(freerdp* instance) IFCALL(instance->PostFinalDisconnect, instance); + freerdp_del_signal_cleanup_handler(instance->context, sig_abort_connect); return rc; } diff --git a/libfreerdp/utils/signal.c b/libfreerdp/utils/signal.c index dc183ddac..d27b44dca 100644 --- a/libfreerdp/utils/signal.c +++ b/libfreerdp/utils/signal.c @@ -28,7 +28,7 @@ #include #include -#define TAG FREERDP_TAG("utils") +#define TAG FREERDP_TAG("utils.signal") #ifdef _WIN32 @@ -38,6 +38,15 @@ int freerdp_handle_signals(void) return -1; } +BOOL freerdp_add_signal_cleanup_handler(void* context, void (*fkt)(void*)) +{ + return FALSE; +} + +BOOL freerdp_del_signal_cleanup_handler(void* context, void (*fkt)(void*)) +{ + return FALSE; +} #else #include @@ -45,8 +54,62 @@ int freerdp_handle_signals(void) volatile sig_atomic_t terminal_needs_reset = 0; int terminal_fildes = 0; -struct termios orig_flags; -struct termios new_flags; +struct termios orig_flags = { 0 }; +struct termios new_flags = { 0 }; + +static BOOL handlers_registered = FALSE; +static pthread_mutex_t signal_handler_lock = PTHREAD_MUTEX_INITIALIZER; + +typedef struct +{ + void* context; + freerdp_signal_handler_t handler; +} cleanup_handler_t; + +static size_t cleanup_handler_count = 0; +static cleanup_handler_t cleanup_handlers[20] = { 0 }; + +static void lock(void) +{ + const int rc = pthread_mutex_lock(&signal_handler_lock); + if (rc != 0) + WLog_ERR(TAG, "[pthread_mutex_lock] failed with %s [%d]", strerror(rc), rc); +} + +static void unlock(void) +{ + const int rc = pthread_mutex_unlock(&signal_handler_lock); + if (rc != 0) + WLog_ERR(TAG, "[pthread_mutex_lock] failed with %s [%d]", strerror(rc), rc); +} + +static void term_handler(int signum) +{ + struct sigaction default_sigaction; + sigset_t this_mask; + static BOOL recursive = FALSE; + + if (!recursive) + { + recursive = TRUE; + WLog_ERR(TAG, "Caught signal '%s' [%d]", strsignal(signum), signum); + } + + if (terminal_needs_reset) + tcsetattr(terminal_fildes, TCSAFLUSH, &orig_flags); + + lock(); + for (size_t x = 0; x < cleanup_handler_count; x++) + { + const cleanup_handler_t empty = { 0 }; + cleanup_handler_t* cur = &cleanup_handlers[x]; + if (cur->handler) + cur->handler(signum, strsignal(signum), cur->context); + *cur = empty; + } + cleanup_handler_count = 0; + unlock(); +} static void fatal_handler(int signum) { @@ -74,9 +137,10 @@ static void fatal_handler(int signum) raise(signum); } +static const int term_signals[] = { SIGINT, SIGKILL, SIGQUIT, SIGSTOP, SIGTERM }; + static const int fatal_signals[] = { SIGABRT, SIGALRM, SIGBUS, SIGFPE, SIGHUP, SIGILL, - SIGINT, SIGKILL, SIGQUIT, SIGSEGV, SIGSTOP, SIGTERM, - SIGTSTP, SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, + SIGSEGV, SIGTSTP, SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, #ifdef SIGPOLL SIGPOLL, #endif @@ -92,34 +156,108 @@ static const int fatal_signals[] = { SIGABRT, SIGALRM, SIGBUS, SIGFPE, SIGHU #endif SIGXCPU, SIGXFSZ }; -int freerdp_handle_signals(void) +static BOOL register_handlers(const int* signals, size_t count, void (*handler)(int)) { - size_t signal_index; - sigset_t orig_set; - struct sigaction orig_sigaction; - struct sigaction fatal_sigaction; - WLog_DBG(TAG, "Registering signal hook..."); - sigfillset(&(fatal_sigaction.sa_mask)); - sigdelset(&(fatal_sigaction.sa_mask), SIGCONT); - pthread_sigmask(SIG_BLOCK, &(fatal_sigaction.sa_mask), &orig_set); - fatal_sigaction.sa_handler = fatal_handler; - fatal_sigaction.sa_flags = 0; + WINPR_ASSERT(signals || (count == 0)); + WINPR_ASSERT(handler); - for (signal_index = 0; signal_index < ARRAYSIZE(fatal_signals); signal_index++) + sigset_t orig_set = { 0 }; + struct sigaction saction = { 0 }; + + pthread_sigmask(SIG_BLOCK, &(saction.sa_mask), &orig_set); + + sigfillset(&(saction.sa_mask)); + sigdelset(&(saction.sa_mask), SIGCONT); + + saction.sa_handler = handler; + saction.sa_flags = 0; + + for (size_t x = 0; x < count; x++) { - if (sigaction(fatal_signals[signal_index], NULL, &orig_sigaction) == 0) + struct sigaction orig_sigaction = { 0 }; + if (sigaction(signals[x], NULL, &orig_sigaction) == 0) { if (orig_sigaction.sa_handler != SIG_IGN) { - sigaction(fatal_signals[signal_index], &fatal_sigaction, NULL); + sigaction(signals[x], &saction, NULL); } } } pthread_sigmask(SIG_SETMASK, &orig_set, NULL); + + return TRUE; +} + +int freerdp_handle_signals(void) +{ + int rc = -1; + + lock(); + + WLog_DBG(TAG, "Registering signal hook..."); + + if (!register_handlers(fatal_signals, ARRAYSIZE(fatal_signals), fatal_handler)) + goto fail; + if (!register_handlers(term_signals, ARRAYSIZE(term_signals), term_handler)) + goto fail; + /* Ignore SIGPIPE signal. */ signal(SIGPIPE, SIG_IGN); - return 0; + handlers_registered = TRUE; + rc = 0; +fail: + unlock(); + return rc; +} + +BOOL freerdp_add_signal_cleanup_handler(void* context, freerdp_signal_handler_t handler) +{ + BOOL rc = FALSE; + lock(); + if (handlers_registered) + { + if (cleanup_handler_count < ARRAYSIZE(cleanup_handlers)) + { + cleanup_handler_t* cur = &cleanup_handlers[cleanup_handler_count++]; + cur->context = context; + cur->handler = handler; + } + else + WLog_WARN(TAG, "Failed to register cleanup handler, only %" PRIuz " handlers supported", + ARRAYSIZE(cleanup_handlers)); + } + rc = TRUE; + unlock(); + return rc; +} + +BOOL freerdp_del_signal_cleanup_handler(void* context, freerdp_signal_handler_t handler) +{ + BOOL rc = FALSE; + lock(); + if (handlers_registered) + { + for (size_t x = 0; x < cleanup_handler_count; x++) + { + cleanup_handler_t* cur = &cleanup_handlers[x]; + if ((cur->context == context) && (cur->handler == handler)) + { + const cleanup_handler_t empty = { 0 }; + for (size_t y = x + 1; y < cleanup_handler_count - 1; y++) + { + *cur++ = cleanup_handlers[y]; + } + + *cur = empty; + cleanup_handler_count--; + break; + } + } + } + rc = TRUE; + unlock(); + return rc; } #endif