On Tue, 2008-01-08 at 22:02 -0500, Jonathan Pryor wrote:
> Thank you for the background on why signal handlers can't be made to
> work with the current Stdlib.signal implementation.
>
> However...
> I don't see why we need a new API to support this. It seems that we
> could retrofit the existing Stdlib.signal() API to use the
> implementation you described, with one difference: a runtime internal
> thread could be used to block on the internal pipe, and when the pipe is
> readable it could dispatch the appropriate signal via delegate
> registered with Stdlib.signal(). No polling, and no new public API
> would be required.
>
> The one thing I'm not sure about is which thread should wait on the
> pipe. Would a dedicated Mono.Posix-internal thread be more appropriate,
> or would one of the existing threads be beter (e.g. a ThreadPool thread,
> or a Timer-related thread, or...).
Attached are patches to mcs/class/Mono.Posix/Mono.Unix.Native and
mono/support as an initial implementation of this idea. It currently
uses a dedicated Mono.Posix-internal thread to do managed signal
dispatching (as the ThreadPool is intended for short-lived tasks, and
I'm not familiar enough with the Timer-related infrastructure to see if
it would be a good fit).
Thoughts?
- Jon
Index: Stdlib.cs
===================================================================
--- Stdlib.cs (revision 92060)
+++ Stdlib.cs (working copy)
@@ -31,6 +31,9 @@
using System.IO;
using System.Runtime.InteropServices;
using System.Text;
+using System.Threading;
+
+using Mono.Unix;
using Mono.Unix.Native;
namespace Mono.Unix.Native {
@@ -359,7 +362,7 @@
// This is the case for using NativeConvert, which will throw an
// exception if an invalid/unsupported value is used.
//
- public class Stdlib
+ public unsafe class Stdlib
{
internal const string LIBC = "msvcrt";
internal const string MPH = "MonoPosixHelper";
@@ -445,24 +448,21 @@
registered_signals = new SignalHandler [(int) signals.GetValue (signals.Length-1)];
}
- [DllImport (LIBC, CallingConvention=CallingConvention.Cdecl,
- SetLastError=true, EntryPoint="signal")]
+ [DllImport (MPH, CallingConvention=CallingConvention.Cdecl,
+ SetLastError=true, EntryPoint="Mono_Posix_Stdlib_signal")]
private static extern IntPtr sys_signal (int signum, SignalHandler handler);
- [DllImport (LIBC, CallingConvention=CallingConvention.Cdecl,
- SetLastError=true, EntryPoint="signal")]
+ [DllImport (MPH, CallingConvention=CallingConvention.Cdecl,
+ SetLastError=true, EntryPoint="Mono_Posix_Stdlib_signal")]
private static extern IntPtr sys_signal (int signum, IntPtr handler);
[CLSCompliant (false)]
public static SignalHandler signal (Signum signum, SignalHandler handler)
{
+ InitSignalSupport ();
+
int _sig = NativeConvert.FromSignum (signum);
- Delegate[] handlers = handler.GetInvocationList ();
- for (int i = 0; i < handlers.Length; ++i) {
- Marshal.Prelink (handlers [i].Method);
- }
-
lock (registered_signals) {
registered_signals [(int) signum] = handler;
}
@@ -494,6 +494,45 @@
#endif
}
+ [DllImport (MPH, CallingConvention=CallingConvention.Cdecl,
+ SetLastError=true, EntryPoint="_mph_set_signal_write_fd")]
+ private static extern void set_signal_write_fd (int signum);
+
+ static object signal_dispatcher;
+ static int signal_read_fd;
+
+ private static void InitSignalSupport ()
+ {
+ if (Path.DirectorySeparatorChar == '\\')
+ return;
+
+ if (signal_dispatcher == null) {
+ object c = new Thread (SignalDispatcher);
+ Thread.MemoryBarrier ();
+ while (Interlocked.CompareExchange (ref signal_dispatcher, c, null) == null) {
+ int writing;
+ if (Syscall.pipe (out signal_read_fd, out writing) < 0)
+ throw UnixMarshal.CreateExceptionForLastError ();
+ set_signal_write_fd (writing);
+ Thread _c = (Thread) c;
+ _c.IsBackground = true;
+ _c.Name = "Mono.Unix.Native Signal Dispatcher";
+ _c.Start ();
+ }
+ Thread.MemoryBarrier ();
+ }
+ }
+
+ private static unsafe void SignalDispatcher ()
+ {
+ byte c;
+ while (Syscall.read (signal_read_fd, &c, 1) >= 1) {
+ SignalHandler h = registered_signals [c];
+ if (h != null)
+ h (c);
+ }
+ }
+
[DllImport (LIBC, CallingConvention=CallingConvention.Cdecl, EntryPoint="raise")]
private static extern int sys_raise (int sig);
Index: ChangeLog
===================================================================
--- ChangeLog (revision 92274)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2008-01-09 Jonathan Pryor <[EMAIL PROTECTED]>
+
+ * Stdlib.cs: Update Stdlib.signal() so that it's safe; see also:
+ http://lists.ximian.com/pipermail/mono-devel-list/2008-January/026501.html
+ http://lists.ximian.com/pipermail/mono-devel-list/2008-January/026503.html
+
2008-01-05 Jonathan Pryor <[EMAIL PROTECTED]>
* Syscall.cs: Add ST_NOEXEC, ST_REMOUNT, ST_BIND to MountFlags. Patch from
Index: ChangeLog
===================================================================
--- ChangeLog (revision 92275)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2008-01-09 Jonathan Pryor <[EMAIL PROTECTED]>
+
+ * signal.c: Provide a safe version of signal(3), as it isn't safe for
+ marshaled delegates to be invoked from signal handler context.
+ * map.h: Flush (adds signal(3)-related prototypes).
+
2008-01-05 Jonathan Pryor <[EMAIL PROTECTED]>
* map.h, map.c: Flush; add new ST_NOEXEC, ST_REMOUNT, and ST_BIND MountFlags
Index: signal.c
===================================================================
--- signal.c (revision 92060)
+++ signal.c (working copy)
@@ -9,6 +9,10 @@
#include <signal.h>
+#ifndef PLATFORM_WIN32
+#include <unistd.h>
+#endif
+
#include "map.h"
#include "mph.h"
@@ -49,6 +53,40 @@
psignal (sig, s);
return errno == 0 ? 0 : -1;
}
+
+static int signal_write;
+
+void
+_mph_set_signal_write_fd (int fd)
+{
+ signal_write = fd;
+}
+
+static void
+default_signal_handler (int signum)
+{
+ // FIXME: currently we assume that all signums are < 255
+ char c = signum;
+ write (signal_write, &c, 1);
+}
+
+void*
+Mono_Posix_Stdlib_signal (int signum, void* handler)
+{
+ if (handler == SIG_DFL || handler == SIG_ERR || handler == SIG_IGN) {
+ return signal (signum, handler);
+ }
+ return signal (signum, default_signal_handler);
+}
+
+#else /* ndef PLATFORM_WIN32 */
+
+void*
+Mono_Posix_Stdlib_signal (int signum, void* handler)
+{
+ return signal (signum, handler);
+}
+
#endif /* ndef PLATFORM_WIN32 */
Index: map.h
===================================================================
--- map.h (revision 92275)
+++ map.h (working copy)
@@ -1397,6 +1397,7 @@
* Delegate Declarations
*/
+typedef void (*SignalHandler) (int signal);
/*
* Structures
@@ -1545,6 +1546,7 @@
/*
* Functions
*/
+void _mph_set_signal_write_fd (int signum);
char* helper_Mono_Posix_GetGroupName (int gid);
char* helper_Mono_Posix_GetUserName (int uid);
char* helper_Mono_Posix_readdir (void* dir);
@@ -1587,6 +1589,7 @@
void* Mono_Posix_Stdlib_SIG_DFL (void);
void* Mono_Posix_Stdlib_SIG_ERR (void);
void* Mono_Posix_Stdlib_SIG_IGN (void);
+void* Mono_Posix_Stdlib_signal (int signum, void* handler);
void* Mono_Posix_Stdlib_stderr (void);
void* Mono_Posix_Stdlib_stdin (void);
void* Mono_Posix_Stdlib_stdout (void);
_______________________________________________
Mono-devel-list mailing list
[email protected]
http://lists.ximian.com/mailman/listinfo/mono-devel-list
