[Mono-dev] Control-C handler

[Paolo Molaro]

On 01/24/08 Jonathan Pryor wrote:
> After talking on IRC, the sane interface is to provide
> Stdlib.signal_default(), Stdlib.signal_error(), and
> Stdlib.signal_ignore() methods instead of using Stdlib.signal() with

Well, those methods shouldn't be in that namespace and the names
are not really nice.
Ignore maps well to a static Ignore (int signal) method but
I can't come up with a nice name for the other two cases.
An alternative is:
	enum SignalAction {
		Default,
		Ignore,
		Error
	}

and
	static int SetSignalAction (int signum, SignalAction action);

> Index: Mono.Unix/UnixSignal.cs
> ===================================================================
> --- Mono.Unix/UnixSignal.cs	(revision 0)
> +++ Mono.Unix/UnixSignal.cs	(revision 0)
> +using System;
> +using System.Runtime.InteropServices;
> +using System.Threading;
> +
> +using Mono.Unix.Native;
> +
> +namespace Mono.Unix {
> +	public class UnixSignal : WaitHandle {

It should have a finalizer.

> +		private Signum signum;
> +		private int _signum;
> +		private IntPtr signal_info;
> +
> +		public UnixSignal (Signum signum)
> +		{
> +			this.signum = signum;
> +			this._signum = NativeConvert.FromSignum (signum);

There is no need to store reduntant info.

> +		private void AssertValid ()
> +		{
> +			if (signal_info == IntPtr.Zero)
> +				throw new ObjectDisposedException (GetType().FullName);
> +		}
> +
> +		private unsafe SignalInfo* Info {
> +			get {return (SignalInfo*) signal_info;}
> +		}
> +
> +		public bool IsSet {
> +			get {
> +				AssertValid ();
> +				return Count > 0;
> +			}
> +		}
> +
> +		public unsafe bool Reset ()
> +		{
> +			AssertValid ();
> +			int n = Info->count;
> +			Info->count = 0;
> +			return n != 0;
> +		}
> +
> +		public unsafe int Count {
> +			get {return Info->count;}
> +			set {Info->count = value;}

Crashes when Info is NULL.

> +		#region WaitHandle overrides
> +		protected unsafe override void Dispose (bool disposing)
> +		{
> +			if (signal_info == IntPtr.Zero)
> +				return;
> +			uninstall (_signum);
> +			signal_info = IntPtr.Zero;
> +		}

This doesn't implement the documented Dispose pattern.

> Index: signal.c
> ===================================================================
> --- signal.c	(revision 92060)
> +++ signal.c	(working copy)
> @@ -3,18 +3,29 @@
>   *
>   * Authors:
>   *   Jonathan Pryor (jonpryor at vt.edu)
> + *   Jonathan Pryor (jpryor at novell.com)
>   *
>   * Copyright (C) 2004-2005 Jonathan Pryor
> + * Copyright (C) 2008 Novell, Inc.
>   */
>  
>  #include <signal.h>
>  
> +#ifndef PLATFORM_WIN32
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <poll.h>

As reported already the last time, you can't unconditionally use poll.

> @@ -49,6 +61,158 @@
>  	psignal (sig, s);
>  	return errno == 0 ? 0 : -1;
>  }
> +
> +static signal_info signals[64];
> +
> +static inline signal_info*
> +get_info (int signum)
> +{
> +	if (signum < 0 || signum >= sizeof(signals)/sizeof(signals [0]))
> +		return NULL;
> +	return &signals [signum];
> +}
> +
> +static void
> +default_handler (int signum)
> +{
> +	signal_info* h = get_info (signum);
> +	int fd = 0;
> +	if (h) {
> +		++h->count;
> +		if (h->write_fd > 0)
> +			fd = h->write_fd;
> +	}
> +	if (fd) {
> +		char c = signum;
> +		write (fd, &c, 1);
> +	}
> +}
> +
> +static pthread_mutex_t signals_mutex = PTHREAD_MUTEX_INITIALIZER;
> +
> +void*
> +Mono_Unix_UnixSignal_install (int sig)
> +{
> +	int mr;
> +	signal_info* h; 
> +
> +	mr = pthread_mutex_lock (&signals_mutex);
> +	if (mr != 0) {
> +		errno = mr;
> +		return NULL;
> +	}
> +
> +	h = get_info (sig);
> +	if (h == NULL)
> +		errno = EINVAL;
> +	else if (h->have_handler)
> +		errno = EADDRINUSE;
> +	else {
> +		h->count = 0;
> +		h->handler = signal (sig, default_handler);
> +		if (h->handler == SIG_ERR) {
> +			h->handler = NULL;
> +		}
> +		else
> +			h->have_handler = 1;
> +	}
> +
> +	pthread_mutex_unlock (&signals_mutex);
> +
> +	return h;
> +}
> +
> +int
> +Mono_Unix_UnixSignal_uninstall (int sig)

It is important (as in my initialy API sketch) that this function take
the signal_info and not the dignal number, as this implementation allows
only just an handler per signal in the API.

lupus

-- 
-----------------------------------------------------------------
lupus at debian.org                                     debian/rules
lupus at ximian.com                             Monkeys do it better