[Mono-dev] FW: RealTimeSignal patch
tim.jenks at realtimeworlds.com
tim.jenks at realtimeworlds.com
Tue Dec 16 10:54:27 EST 2008
Hi List & Jon,
The attached patches to the class libraries (Mono.Posix) & native posix support code (signal.c) provides support for handling SIGRT* signals in the Mono.Posix libraries, specifically:
* A RealTimeSignum struct for representing realtime signals, providing an offset to SIGRTMIN
** MinValue and MaxValue provide the minimum and maximum signal values (SIGRTMIN and SIGRTMAX)
* NativeConvert static methods provide:
** Conversion between an offset to SIGRTMIN and a RealTimeSignum
** Conversion between a RealTimeSignum and a platform specific SIGRT* signal number
* A new UnixSignal constructor overload for handling RealTimeSignum
** Added new IsRealTimeSignal property to indicate whether a UnixSignal represents a realtime signal
* Stdlib.raise and Stdlib.SetSigAction overloads for RealTimeSignum
I've also addressed Jon's comments in this second pass today which were from a discussion off-list. Quoted is snippets of yesterdays patch for reference :)
Cheers
-Tim
-----Original Message-----
From: Jonathan Pryor [mailto:jonpryor at vt.edu]
Sent: 15 December 2008 21:25
To: Tim Jenks
Subject: Re: RealTimeSignal patch
Offhand, which is better: RealTimeSignal or RealTimeSignum? Given that
it's effectively a variant of the Signum values, I think the latter may
be better...
Also, you should post follow ups to mono-devel-list.
On Mon, 2008-12-15 at 14:10 +0000, tim.jenks at realtimeworlds.com wrote:
> Index: class/Mono.Posix/Mono.Unix.Native/Stdlib.cs
> ===================================================================
> --- class/Mono.Posix/Mono.Unix.Native/Stdlib.cs (revision 121281)
> +++ class/Mono.Posix/Mono.Unix.Native/Stdlib.cs (working copy)
> @@ -531,9 +541,20 @@
> public static int raise (Signum sig)
> {
> int _sig = NativeConvert.FromSignum (sig);
> - return sys_raise (_sig);
> + return raise(_sig);
> }
>
> + public static int raise (RealTimeSignal rts)
> + {
> + int _sig = NativeConvert.FromRealTimeSignal(rts);
> + return raise(_sig);
> + }
> +
> + private static int raise(int signum)
> + {
> + return sys_raise(signum);
> + }
Remove the raise(int) overload, and just have raise(Signum) and
raise(RealTimeSignal) call sys_raise() directly.
> Index: class/Mono.Posix/Mono.Unix.Native/NativeConvert.cs
> ===================================================================
> --- class/Mono.Posix/Mono.Unix.Native/NativeConvert.cs (revision 121281)
> +++ class/Mono.Posix/Mono.Unix.Native/NativeConvert.cs (working copy)
> @@ -18,6 +18,18 @@
> // Non-generated exports
> //
>
> + [DllImport (LIB, EntryPoint="Mono_Posix_FromRealTimeSignal")]
> + private static extern int FromRealTimeSignal (Int32 offset, out Int32 rval);
`private` should line up with the [DllImport] on the previous line.
> +
> + // convert a realtime signal to os signal
> + public static int FromRealTimeSignal(RealTimeSignal sig)
> + {
> + int sigNum;
> + if (FromRealTimeSignal(sig.Offset, out sigNum) == -1)
> + ThrowArgumentException(sig.Offset);
> + return sigNum;
> + }
> +
Should we also provide a NativeConvert.ToRealTimeSignal(int) method? It
would merely call the RealTimeSignal constructor, but it may make
members easier to find.
> Index: class/Mono.Posix/Test/Mono.Unix/UnixSignalTest.cs
> ===================================================================
> --- class/Mono.Posix/Test/Mono.Unix/UnixSignalTest.cs (revision 121281)
> +++ class/Mono.Posix/Test/Mono.Unix/UnixSignalTest.cs (working copy)
> @@ -3,11 +3,13 @@
> //
> // Authors:
> // Jonathan Pryor <jonpryor at vt.edu>
> +// Tim Jenks <tim.jenks at realtimeworlds.com>
> //
> // (C) 2008 Jonathan Pryor
> //
>
> using NUnit.Framework;
> +using NUnit.Framework.SyntaxHelpers;
> using System;
> using System.Text;
> using System.Threading;
> @@ -18,7 +20,88 @@
>
> [TestFixture]
> public class UnixSignalTest {
> +
> [Test]
> + public void TestRealTimeCstor()
> + {
> + RealTimeSignal rts = new RealTimeSignal(0);
> + using (UnixSignal s = new UnixSignal(rts))
> + {
> + Assert.That(s.RealTimeSignal, Is.EqualTo(rts));
> + }
> + }
> +
> + [Test]
> + [ExpectedException]
> + public void TestRealTimeOutOfRange()
> + {
> + RealTimeSignal rts = new RealTimeSignal(int.MaxValue);
> + using (UnixSignal s = new UnixSignal(rts))
> + {
> + }
> + }
> +
> + [Test]
> + public void TestRaiseRTMINSignal()
> + {
> +
> + RealTimeSignal rts = new RealTimeSignal(0);
> +
> + Thread t1 = new Thread(delegate() {
> + using (UnixSignal s = new UnixSignal(rts))
> + {
> + DateTime start = DateTime.Now;
> + bool r = s.WaitOne (5000, false);
> + DateTime end = DateTime.Now;
> + Assert.AreEqual (s.Count, 1);
> + Assert.AreEqual (r, true);
> + if ((end - start) > new TimeSpan (0, 0, 5))
> + throw new InvalidOperationException ("Signal slept too long");
> + if ((end - start) < new TimeSpan (0, 0, 1))
> + throw new InvalidOperationException ("Signal raised too early");
> + }
> + });
> + Thread t2 = new Thread(delegate() {
> + Thread.Sleep(1000);
> + Stdlib.raise(rts);
> + });
> + t1.Start();
> + t2.Start();
> + t1.Join();
> + t2.Join();
> +
> + }
You should refactor
TestRaiseRTMINSignal(), TestRaiseRTMINPlusOneSignal(), and TestRaise()
to share more code, as they're almost entirely identical.
> Index: class/Mono.Posix/Mono.Unix/UnixSignal.cs
> ===================================================================
> --- class/Mono.Posix/Mono.Unix/UnixSignal.cs (revision 121281)
> +++ class/Mono.Posix/Mono.Unix/UnixSignal.cs (working copy)
> @@ -3,6 +3,7 @@
> //
> // Authors:
> // Jonathan Pryor (jpryor at novell.com)
> +// Tim Jenks (tim.jenks at realtimeworlds.com)
> //
> // (C) 2008 Novell, Inc.
> //
> @@ -33,9 +34,35 @@
> using Mono.Unix.Native;
>
> namespace Mono.Unix {
> +
> + public struct RealTimeSignal
This should also implement IEquatable<RealTimeSignal>, override
GetHashCode(), Eqaus(), and provide operator== and operator!=. See most
of the structures in Mono.Unix.Native/Syscall.cs (e.g. Flock) for
examples, including the `#if NET_2_0` block usage.
> + {
> + [DllImport (Stdlib.MPH, CallingConvention=CallingConvention.Cdecl,
> + EntryPoint="Mono_Posix_SIGRTMIN")]
> + static extern int GetSIGRTMIN();
This should probably be internal to UnixSignal.
> + private int rt_offset;
> +
> + public static int SIGRTMIN
> + {
> + get { return GetSIGRTMIN(); }
> + }
This doesn't follow FxDG naming convention (which Mono.Unix attempts to
do, while Mono.Unix.Native follows Unix convention). I don't think it's
that useful a property type, either; I'd prefer:
public static readonly RealTimeSignal MinValue =
new RealTimeSignal ();
public static readonly RealTimeSignal MaxValue =
new RealTimeSignal (GetSIGRTMAX()-GetSIGRTMIN() - 1);
> +
> + public RealTimeSignal(int offset)
> + {
> + rt_offset = offset;
Should assert that offset >= 0 and <= MaxValue.Offset.
> + }
> +
> + public int Offset
> + {
Braces for properties go on the same line as the property name.
> + get { return rt_offset; }
> + }
> + }
> +
> public class UnixSignal : WaitHandle {
> private Signum signum;
> private IntPtr signal_info;
> + private RealTimeSignal rtsignal;
This shouldn't be necessary; change `signum` into an int, and convert
this.signum into Signum or RealTimeSignal as appropriate. ("as
appropriate" == "if this.signum >= SIGRTMIN, it's a real time signal".)
NativeConvert.ToSignum() can be used within the Signum property to
convert signum into the appropriate enumeration value.
A related question is what should UnixSignal.Signum do if it's a
realtime signal? Throwing an InvalidOperationException may be the only
thing to do, but that isn't particularly nice. Go with the exception
for now. Similarly, the RealTimeSignal property should throw if it's
not a real time signal.
Add a `public bool IsRealTimeSignal {get;}' property so that users can
know which type of signal this is.
> Index: support/signal.c
> ===================================================================
> --- support/signal.c (revision 121281)
> +++ support/signal.c (working copy)
> +int Mono_Posix_FromRealTimeSignal (int offset, int *r)
> +{
> + *r = 0;
> +#ifdef SIGRTMIN
> +#ifdef SIGRTMAX
> + if (SIGRTMIN+offset > SIGRTMAX)
> + return -1;
> + *r = SIGRTMIN+offset;
> + return 0;
> +#else /* def SIGRTMAX */
> + return -1;
> +#endif /* ndef SIGRTMAX */
> +#else /* def SIGRTMAX */
> + return -1;
> +#endif /* ndef SIGRTMAX */
> +}
Or just use:
#if defined (SIGRTMIN) && defined (SIGRTMAX)
if (offset < 0 || SIGRTMIN > SIGRTMAX - offset)
return -1;
*r = SIGRTMIN + offset;
return 0;
#else
return -1;
#endif
- 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mcs.diff
Type: application/octet-stream
Size: 11978 bytes
Desc: mcs.diff
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20081216/fedc7d70/attachment-0002.obj
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mono.diff
Type: application/octet-stream
Size: 793 bytes
Desc: mono.diff
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20081216/fedc7d70/attachment-0003.obj
More information about the Mono-devel-list
mailing list