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

[tim.jenks at realtimeworlds.com]

Hi Jon & List,

Here's another take at this implemented as you suggested.

Couple of questions though:

> 1. Somehow find out which is preferable for "normal" UnixSignal usage:
> 
>   a. One pipe/UnixSignal instance for the lifetime of the UnixSignal.
>   b. One pipe/UnixSignal instance for the lifetime of a WaitAny()
call.
> 
> I'm assuming "normal" is that a UnixSignal will last for the life of
> the
> application; that is, UnixSignal instances won't be short lived.
> Meanwhile, I'd expect WaitAny() to be comparatively infrequent, but
> have
> less clues about how it's used in practice.

We're using SIGRT* raised a bunch of times a second for the update tick
of a game server, I'm talking 20-30Hz. For our use case I think a) is
preferable, though I've implemented this as b) as suggested just now. 

What are your thoughts?

> You might need to change wait_for_any() to actually perform error
> checking on the read(h->read_fd, &c, 1) call, as this could now error
> if
> multiple threads are calling WaitAny().

What kind of error conditions are you expecting in this instance?

And finally, here's some comments on mph_int*:

Obviously the signal handler can be executing concurrently with the
other functions in signal.c that are guarded by the mutex; to keep a
consistent view of pipecnt in the signal handler I have used atomics:

1. There does not appear to be an equivalent to InterlockedIncrement in
the glib functions (returning the value after atomic inc), so there is
code in setup_pipes that would race if it were re-entrant. It isn't
re-entrant due to the global mutex being held but this needs tidying to
avoid confusion. Effectively, I need to: if
(InterlockedIncrement(pipecnt) == 1) { setup pipes }. 

2. Conversely, I needed an atomic decrement. Having looked at the glib
functions I could only find g_atomic_int_dec_and_test in 2.4. Is this
OK, or is there an a g_atomic_int_dec ? This is to achieve: if
(InterlockedDecrement(pipecnt) == 0) { teardown pipes }.
g_atomic_int_dec_and_test clearly does the job, but it's a little
inconsistent with InterlockedDecrement with respect to the mph_int*
macros.

Cheers
-Tim

____________________________________________________________________
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signal.c.diff
Type: application/octet-stream
Size: 3986 bytes
Desc: signal.c.diff
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20090202/04215a1e/attachment.obj 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: UnixSignal.cs.diff
Type: application/octet-stream
Size: 437 bytes
Desc: UnixSignal.cs.diff
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20090202/04215a1e/attachment-0001.obj