[Pidgin] #7902: The sighandler() function in pidgin/gtkmain.c doesn't restore a SIGALRM handler.

[Pidgin]

#7902: The sighandler() function in pidgin/gtkmain.c doesn't restore a SIGALRM
handler.
--------------------------------------+-------------------------------------
 Reporter:  quonsto                   |        Owner:              
     Type:  defect                    |       Status:  new         
Milestone:                            |    Component:  pidgin (gtk)
  Version:  2.5.3                     |   Resolution:              
 Keywords:  pidgin SIGALRM gstreamer  |  
--------------------------------------+-------------------------------------
Description changed by quonsto:

Old description:

> pidgin/gtkmain.c contains code, which looks like this:
>
> {{{
> #if defined(USE_GSTREAMER) && !defined(GST_CAN_DISABLE_FORKING)
> /* ... skipped long comment ... */
>         case SIGCHLD:
>                 /* Restore signal catching */
>                 signal(SIGCHLD, sighandler);
>                 alarm(1);
>                 break;
>         case SIGALRM:
> #else
>         case SIGCHLD:
> #endif
>                 clean_pid();
>                 /* Restore signal catching */
>                 signal(SIGCHLD, sighandler);
>                 break;
> }}}
>
> If this code is compiled with macro USE_GSTREAMER defined and
> GST_CAN_DISABLE_FORKING not defined, it may lead to an uncatched SIGALRM
> (and pidgin termination), if the pidgin process receives more than one
> SIGCHLD during its lifetime.
>
> The problem is a SIGALRM handler which erroneously restores SIGCHLD
> handler instead of SIGALRM.
>
> The code after '#endif' in this piece of code is good for the case when
> being compiled without USE_GSTREAMER macro, or with
> GST_CAN_DISABLE_FORKING. [[BR]]
> But it doesn't suit otherwise.
>
> The proposed patch is to completely split the mentioned piece of code
> into two cases: one for "defined(USE_GSTREAMER) &&
> !defined(GST_CAN_DISABLE_FORKING)" and the other for the opposite case.
>
> A variant of the patch, implemeting this idea is given below:
>
> {{{
>
> --- gtkmain.c.orig      2008-12-21 03:38:16.000000000 +0300
> +++ gtkmain.c   2008-12-25 20:51:57.949286800 +0300
> @@ -207,13 +207,17 @@
>                 alarm(1);
>                 break;
>         case SIGALRM:
> +               clean_pid();
> +               /* Restore signal catching */
> +               signal(SIGALRM, sighandler);
> +               break;
>  #else
>         case SIGCHLD:
> -#endif
>                 clean_pid();
>                 /* Restore signal catching */
>                 signal(SIGCHLD, sighandler);
>                 break;
> +#endif
>         default:
>                 purple_debug_warning("sighandler", "Caught signal %d\n",
> sig);
>                 purple_core_quit();
>
> }}}
>

> Best regards, K.S.

New description:

 The sighandler() function in pidgin/gtkmain.c contains code, which looks
 like this:

 {{{
 #if defined(USE_GSTREAMER) && !defined(GST_CAN_DISABLE_FORKING)
 /* ... skipped long comment ... */
         case SIGCHLD:
                 /* Restore signal catching */
                 signal(SIGCHLD, sighandler);
                 alarm(1);
                 break;
         case SIGALRM:
 #else
         case SIGCHLD:
 #endif
                 clean_pid();
                 /* Restore signal catching */
                 signal(SIGCHLD, sighandler);
                 break;
 }}}

 If this code is compiled with macro USE_GSTREAMER defined and
 GST_CAN_DISABLE_FORKING not defined, it may lead to an uncatched SIGALRM
 (and pidgin termination), if the pidgin process receives more than one
 SIGCHLD during its lifetime.

 The problem is a SIGALRM handler which erroneously restores SIGCHLD
 handler instead of SIGALRM.

 The code after '#endif' in this piece of code is good for the case when
 being compiled without USE_GSTREAMER macro, or with
 GST_CAN_DISABLE_FORKING. [[BR]]
 But it doesn't suit otherwise.

 The proposed patch is to completely split the mentioned piece of code into
 two cases: one for "defined(USE_GSTREAMER) &&
 !defined(GST_CAN_DISABLE_FORKING)" and the other for the opposite case.

 A variant of the patch, implemeting this idea is given below:

 {{{

 --- gtkmain.c.orig      2008-12-21 03:38:16.000000000 +0300
 +++ gtkmain.c   2008-12-25 20:51:57.949286800 +0300
 @@ -207,13 +207,17 @@
                 alarm(1);
                 break;
         case SIGALRM:
 +               clean_pid();
 +               /* Restore signal catching */
 +               signal(SIGALRM, sighandler);
 +               break;
  #else
         case SIGCHLD:
 -#endif
                 clean_pid();
                 /* Restore signal catching */
                 signal(SIGCHLD, sighandler);
                 break;
 +#endif
         default:
                 purple_debug_warning("sighandler", "Caught signal %d\n",
 sig);
                 purple_core_quit();

 }}}


 Best regards, K.S.

--

-- 
Ticket URL: <http://developer.pidgin.im/ticket/7902#comment:1>
Pidgin <http://pidgin.im>
Pidgin