[Mono-dev] signal.c cross-thread access

[tim.jenks at realtimeworlds.com]

The test for >64 signals passed to WaitAny, and the bounds check in the
WaitAny entry point, is to guard against overrunning the bounds of a new
pollfd array of NUM_SIGNALS length:

+       struct pollfd fd_structs[NUM_SIGNALS];
+       if (count > NUM_SIGNALS)

I don't have an account to commit, so if you could do the honours if you
are happy with the bounds check.

Cheers
-Tim

> -----Original Message-----
> From: Jonathan Pryor [mailto:jonpryor at vt.edu]
> Sent: 19 February 2009 15:53
> To: Tim Jenks
> Cc: mono-devel-list at lists.ximian.com
> Subject: Re: [Mono-dev] signal.c cross-thread access
> 
> It's looking good, just one question remains.
> 
> On Thu, 2009-02-12 at 14:51 +0000, tim.jenks at realtimeworlds.com wrote:
> Index: Test/Mono.Unix/UnixSignalTest.cs
> > ===================================================================
> > --- Test/Mono.Unix/UnixSignalTest.cs    (revision 123183)
> > +++ Test/Mono.Unix/UnixSignalTest.cs    (working copy)
> > +               [Test]
> > +               public void TestWaitAnyFailsWithMore64Signals()
> > +               {
> > +                       UnixSignal s1 = new
> UnixSignal(Signum.SIGINT);
> > +                       UnixSignal[] signals = new UnixSignal[65];
> > +                       for (int i=0; i<65; ++i)
> > +                               signals[i] = s1;
> > +
> > +                       Assert.That(UnixSignal.WaitAny(signals, new
> TimeSpan(0,0,1)), Is.EqualTo(-1));
> > +               }
> 
> Why should this be an error?  The "no more than 64 UnixSignal
> instances"
> is a restriction on the number of UnixSignal instances, not the number
> we can block on...
> 
> > Index: signal.c
> > ===================================================================
> > --- signal.c    (revision 123183)
> > +++ signal.c    (working copy)
> > @@ -333,27 +370,32 @@
> >  int
> >  Mono_Unix_UnixSignal_WaitAny (void** _signals, int count, int
> timeout /* milliseconds */)
> >  {
> > -       fd_set read_fds;
> > -       int mr, r;
> > -       int max_fd = 0;
> > +       int r;
> > +       int currfd = 0;
> > +       struct pollfd fd_structs[NUM_SIGNALS];
> >
> >         signal_info** signals = (signal_info**) _signals;
> >
> > -       mr = pthread_mutex_lock (&signals_mutex);
> > -       if (mr != 0) {
> > -               errno = mr;
> > +       if (count > NUM_SIGNALS)
> >                 return -1;
> 
> Again, I don't understand the need for this check.
> 
> Otherwise, this is looking good.  Do you have an account to commit
> with,
> or will I need to do it?
> 
> Thanks,
>  - Jon
> 
> 
> 
> ____________________________________________________________________
> This email has been scanned by the MessageLabs Email Security System

____________________________________________________________________
DISCLAIMER

This message and any attachments contain privileged and confidential information intended for the use of the addressee named above. If you are not the intended recipient of this message, you are hereby notified that any use, dissemination, distribution or reproduction of this message is prohibited. Please note that we cannot guarantee that this message or any attachment is virus free or that it has not been intercepted and amended. The views of the author may not necessarily reflect those of Realtime Worlds Ltd.

 

Realtime Worlds Ltd is registered in Scotland, number 225628. Registered Office: 152 West Marketgait, Dundee, DD1 1NJ.