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

[Jonathan Pryor]

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