bugmake - Bugs: bug #41341, Race condition in job.c...

 
 

bug #41341: Race condition in job.c set_child_handler_action_flags - alarm is set before SIGALRM handler

Submitter:  None
Submitted:  Sat 25 Jan 2014 12:38:16 AM UTC
   
 
Severity:  3 - Normal Priority:  5 - Normal
Item Group:  Bug Status:  Fixed
Privacy:  Public Assigned to:  psmith
Open/Closed:  Closed Component Version:  4.0
Operating System:  None Fixed Release:  4.1
Triage Status:  None
* Mandatory Fields

Post a Comment

Add a New Comment Rich Markup
   

Discussion

Sun 02 Feb 2014 04:12:40 AM UTC, comment #1: 

Thank you; this fix will be in the next release of GNU make.

Paul D. Smith <psmith>
Group administrator
Sat 25 Jan 2014 12:38:16 AM UTC, original submission:  

Hi, there is possible race condition in job.c in function set_child_handler_action_flags when it is called to set alarm. There is short period of time, when alarm is set, but handler is not set; sometimes this may kill make process.

The code fragment:

http://git.savann ... /tree/job.c#n1093

set_child_handler_action_flags (int set_handler, int set_alarm)
{
....
#if defined SIGALRM
  if (set_alarm)
    {
      /* If we're about to enter the read(), set an alarm to wake up in a
         second so we can check if the load has dropped and we can start more
         work.  On the way out, turn off the alarm and set SIG_DFL.  */
      alarm (set_handler ? 1 : 0);
      sa.sa_handler = set_handler ? job_noop : SIG_DFL;
      sa.sa_flags = 0;
      sigaction (SIGALRM, &sa, NULL);
    }
#endif
}
#endif

If we are setting alarm, the order of calls is:

 alarm(1);
 sa.sa_handler=job_noop;
 sigaction(SIGALRM, &sa, NULL);

And when we are unsetting the alarm, the order is

 alarm(0);
 sa.sa_handler=SIG_DFL; // DFL is kill
 sigaction(SIGALRM, &sa, NULL);

So, for the setting of alarm, when used on very slow machine or on very loaded machine there is probability that make process will do alarm(1) and then will be descheduled from CPU for time of more than 1 real time second. SIGALRM will be generated and delivered before sigaction call, killing the make process.

I think the better way of setting the alarm is to register handler BEFORE setting alarm and deregister it AFTER disabling alarm, like

if(set_handler) {
      sa.sa_handler = job_noop;
      sa.sa_flags = 0;
      sigaction (SIGALRM, &sa, NULL);
      alarm (1);
} else {
      alarm (0);
      sa.sa_handler = SIG_DFL;
      sa.sa_flags = 0;
      sigaction (SIGALRM, &sa, NULL);
}

Thanks!

Anonymous

 

Attached Files

This item currently has no attached files.

(Note: upload size limit is set to 4.0MiB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

 

Dependencies

This item does not depend on any other items.

No items depend on this one.

 

Mail Notification Carbon-Copy List

Carbon-Copy List
  • -email is unavailable- added by psmith (Posted a comment)
  •  

    Votes

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    History

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-02-02 psmith StatusNone Fixed
        Assigned toNone psmith
        Open/ClosedOpen Closed
        Fixed ReleaseNone 4.1

    Back to the top

    Powered by Savane 3.16-ed84.
    Corresponding source code