> > Hi, > > On Thu, May 23, 2019 at 3:31 PM Marc-André Lureau > <marcandre.lureau@xxxxxxxxx> wrote: > > > > Hi > > > > On Thu, May 23, 2019 at 10:37 AM Jakub Janků <jjanku@xxxxxxxxxx> wrote: > > > > > > According to [0], g_debug should not be used in a signal handler. > > > So, to avoid reentrancy, do not print debug message when quit is > > > called with SIGINT. > > > > > > [0] > > > https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/signal?view=vs-2019 > > > ???? The quit function is called by signal handler or "manually". If called manually it's not a problem. The only signal registered for this function is SIGINT which in Windows is managed by another thread (as written in the link you sent, and by the way is handled by SetConsoleCtrlHandler) so it's not a problem to call g_debug. Note that this function is also called manually with SIGTERM but still not a problem on Windows as service_ctrl_handler is run in another thread. The problems I see is that quit_service should be defined volatile and g_main_loop_quit should not be called on Unix! If a lock used by g_main_loop_quit is retained while the signal is called you'll have a deadlock. Maybe I'm wrong but I didn't find a note if g_main_loop_quit is signal safe so better not to call it from a signal handler. g_unix_signal_add seems a good solution for Unix. > > > Signed-off-by: Jakub Janků <jjanku@xxxxxxxxxx> > > > --- > > > spice/spice-webdavd.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c > > > index e494692..cdfa73d 100644 > > > --- a/spice/spice-webdavd.c > > > +++ b/spice/spice-webdavd.c > > > @@ -237,7 +237,8 @@ static void mdns_unregister_service (void); > > > static void > > > quit (int sig) > > > { > > > - g_debug ("quit %d", sig); > > > + if (sig != SIGINT) > > > + g_debug ("quit %d", sig); > > > > > > > I would simply remove the g_debug() call then. > > Ok then. > > On Unix, we could use g_unix_signal_add, I'll change it. > But sadly there doesn't seem to be a Windows equivalent. > > Cheers, > Jakub > > > > (maybe we should have a different function for the signal handler) > > It sounds a great idea. > > > if (sig == SIGINT || sig == SIGTERM) > > > quit_service = TRUE; Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel