* Added some comments to the thread flags.

* Added new thread flag THREAD_FLAGS_ALWAYS_RESTART_SYSCALL. If set, it forces
  syscall restart even when a signal handler without SA_RESTART was invoked.
* Fixed sigwait(): If one of requested signals wasn't already pending it would
  never wake up. Also, the syscall always needs to be restarted, if interrupted
  by another signal.
* Renamed a bunch of the POSIX signal function implementations which did return
  an error code directly (instead via errno). Added correct POSIX functions
  where needed.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@36054 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/private/kernel/thread_types.h

@@ -348,16 +348,34 @@ struct thread_queue {
 
 // bits for the thread::flags field
 #define	THREAD_FLAGS_SIGNALS_PENDING		0x0001
+	// unblocked signals are pending (computed flag for optimization purposes)
 #define	THREAD_FLAGS_DEBUG_THREAD			0x0002
+	// forces the thread into the debugger as soon as possible (set by
+	// debug_thread())
 #define	THREAD_FLAGS_DEBUGGER_INSTALLED		0x0004
+	// a debugger is installed for the current team (computed flag for
+	// optimization purposes)
 #define	THREAD_FLAGS_BREAKPOINTS_DEFINED	0x0008
+	// hardware breakpoints are defined for the current team (computed flag for
+	// optimization purposes)
 #define	THREAD_FLAGS_BREAKPOINTS_INSTALLED	0x0010
+	// breakpoints are currently installed for the thread (i.e. the hardware is
+	// actually set up to trigger debug events for them)
 #define	THREAD_FLAGS_64_BIT_SYSCALL_RETURN	0x0020
+	// set by 64 bit return value syscalls
 #define	THREAD_FLAGS_RESTART_SYSCALL		0x0040
+	// set by handle_signals(), if the current syscall shall be restarted
 #define	THREAD_FLAGS_DONT_RESTART_SYSCALL	0x0080
-#define	THREAD_FLAGS_SYSCALL_RESTARTED		0x0100
+	// explicitly disables automatic syscall restarts (e.g. resume_thread())
-#define	THREAD_FLAGS_SYSCALL				0x0200
+#define	THREAD_FLAGS_ALWAYS_RESTART_SYSCALL	0x0100
-	// Note: Set only for certain syscalls.
+	// force syscall restart, even if a signal handler without SA_RESTART was
+	// invoked (e.g. sigwait())
+#define	THREAD_FLAGS_SYSCALL_RESTARTED		0x0200
+	// the current syscall has been restarted
+#define	THREAD_FLAGS_SYSCALL				0x0400
+	// the thread is currently in a syscall; set/reset only for certain
+	// functions (e.g. ioctl()) to allow inner functions to discriminate
+	// whether e.g. parameters where passed from userland or kernel
 
 
 #endif	/* _KERNEL_THREAD_TYPES_H */

src/system/kernel/signal.cpp

@@ -11,6 +11,7 @@
 
 #include <ksignal.h>
 
+#include <errno.h>
 #include <stddef.h>
 #include <string.h>
 
@@ -23,6 +24,7 @@
 #include <kscheduler.h>
 #include <sem.h>
 #include <syscall_restart.h>
+#include <syscall_utils.h>
 #include <team.h>
 #include <thread.h>
 #include <tracing.h>
@@ -331,9 +333,12 @@ handle_signals(struct thread *thread)
 
 	thread->user_thread->pending_signals = 0;
 
-	bool restart = (atomic_and(&thread->flags,
+	uint32 restartFlags = atomic_and(&thread->flags,
-			~THREAD_FLAGS_DONT_RESTART_SYSCALL)
+		~THREAD_FLAGS_DONT_RESTART_SYSCALL);
-		& THREAD_FLAGS_DONT_RESTART_SYSCALL) == 0;
+	bool alwaysRestart
+		= (restartFlags & THREAD_FLAGS_ALWAYS_RESTART_SYSCALL) != 0;
+	bool restart = alwaysRestart
+		|| (restartFlags & THREAD_FLAGS_DONT_RESTART_SYSCALL) == 0;
 
 	T(HandleSignals(signalMask));
 
@@ -475,8 +480,10 @@ handle_signals(struct thread *thread)
 		if (debugSignal && !notify_debugger(thread, signal, handler, false))
 			continue;
 
-		if (!restart || (handler->sa_flags & SA_RESTART) == 0)
+		if (!restart
+				|| ((!alwaysRestart && handler->sa_flags & SA_RESTART) == 0)) {
 			atomic_and(&thread->flags, ~THREAD_FLAGS_RESTART_SYSCALL);
+		}
 
 		T(ExecuteSignalHandler(signal, handler));
 
@@ -702,8 +709,8 @@ has_signals_pending(void *_thread)
 }
 
 
-int
+static int
-sigprocmask(int how, const sigset_t *set, sigset_t *oldSet)
+sigprocmask_internal(int how, const sigset_t *set, sigset_t *oldSet)
 {
 	struct thread *thread = thread_get_current_thread();
 	sigset_t oldMask = atomic_get(&thread->sig_block_mask);
@@ -735,11 +742,18 @@ sigprocmask(int how, const sigset_t *set, sigset_t *oldSet)
 }
 
 
+int
+sigprocmask(int how, const sigset_t *set, sigset_t *oldSet)
+{
+	RETURN_AND_SET_ERRNO(sigprocmask_internal(how, set, oldSet));
+}
+
+
 /*!	\brief sigaction() for the specified thread.
 	A \a threadID is < 0 specifies the current thread.
 */
-int
+static status_t
-sigaction_etc(thread_id threadID, int signal, const struct sigaction *act,
+sigaction_etc_internal(thread_id threadID, int signal, const struct sigaction *act,
 	struct sigaction *oldAction)
 {
 	struct thread *thread;
@@ -792,6 +806,15 @@ sigaction_etc(thread_id threadID, int signal, const struct sigaction *act,
 }
 
 
+int
+sigaction_etc(thread_id threadID, int signal, const struct sigaction *act,
+	struct sigaction *oldAction)
+{
+	RETURN_AND_SET_ERRNO(sigaction_etc_internal(threadID, signal, act,
+		oldAction));
+}
+
+
 int
 sigaction(int signal, const struct sigaction *act, struct sigaction *oldAction)
 {
@@ -854,40 +877,57 @@ set_alarm(bigtime_t time, uint32 mode)
 /*!	Wait for the specified signals, and return the signal retrieved in
 	\a _signal.
 */
-int
+static status_t
-sigwait(const sigset_t *set, int *_signal)
+sigwait_internal(const sigset_t *set, int *_signal)
 {
-	struct thread *thread = thread_get_current_thread();
+	sigset_t requestedSignals = *set & BLOCKABLE_SIGNALS;
 
-	while (!has_signals_pending(thread)) {
+	struct thread* thread = thread_get_current_thread();
-		thread_prepare_to_block(thread, B_CAN_INTERRUPT,
-			THREAD_BLOCK_TYPE_SIGNAL, NULL);
-		thread_block();
-	}
 
-	int signalsPending = atomic_get(&thread->sig_pending) & *set;
+	while (true) {
-
+		sigset_t pendingSignals = atomic_get(&thread->sig_pending);
-	update_current_thread_signals_flag();
+		sigset_t blockedSignals = atomic_get(&thread->sig_block_mask);
-
+		sigset_t pendingRequestedSignals = pendingSignals & requestedSignals;
-	if (signalsPending) {
+		if ((pendingRequestedSignals) != 0) {
-		// select the lowest pending signal to return in _signal
+			// select the lowest pending signal to return in _signal
-		for (int signal = 1; signal < NSIG; signal++) {
+			for (int signal = 1; signal < NSIG; signal++) {
-			if ((SIGNAL_TO_MASK(signal) & signalsPending) != 0) {
+				if ((SIGNAL_TO_MASK(signal) & pendingSignals) != 0) {
-				*_signal = signal;
+					atomic_and(&thread->sig_pending, ~SIGNAL_TO_MASK(signal));
-				return B_OK;
+					*_signal = signal;
+					return B_OK;
+				}
 			}
 		}
-	}
 
-	return B_INTERRUPTED;
+		if ((pendingSignals & ~blockedSignals) != 0) {
+			// Non-blocked signals are pending -- return to let them be handled.
+			return B_INTERRUPTED;
+		}
+
+		// No signals yet. Set the signal block mask to not include the
+		// requested mask and wait until we're interrupted.
+		atomic_set(&thread->sig_block_mask,
+			blockedSignals & ~(requestedSignals & BLOCKABLE_SIGNALS));
+
+		while (!has_signals_pending(thread)) {
+			thread_prepare_to_block(thread, B_CAN_INTERRUPT,
+				THREAD_BLOCK_TYPE_SIGNAL, NULL);
+			thread_block();
+		}
+
+		// restore the original block mask
+		atomic_set(&thread->sig_block_mask, blockedSignals);
+
+		update_current_thread_signals_flag();
+	}
 }
 
 
 /*!	Replace the current signal block mask and wait for any event to happen.
 	Before returning, the original signal block mask is reinstantiated.
 */
-int
+static status_t
-sigsuspend(const sigset_t *mask)
+sigsuspend_internal(const sigset_t *mask)
 {
 	T(SigSuspend(*mask));
 
@@ -918,8 +958,8 @@ sigsuspend(const sigset_t *mask)
 }
 
 
-int
+static status_t
-sigpending(sigset_t *set)
+sigpending_internal(sigset_t *set)
 {
 	struct thread *thread = thread_get_current_thread();
 
@@ -961,7 +1001,7 @@ _user_sigprocmask(int how, const sigset_t *userSet, sigset_t *userOldSet)
 				sizeof(sigset_t)) < B_OK))
 		return B_BAD_ADDRESS;
 
-	status = sigprocmask(how, userSet ? &set : NULL,
+	status = sigprocmask_internal(how, userSet ? &set : NULL,
 		userOldSet ? &oldSet : NULL);
 
 	// copy old set if asked for
@@ -1009,9 +1049,14 @@ _user_sigwait(const sigset_t *userSet, int *_userSignal)
 		return B_BAD_ADDRESS;
 
 	int signal;
-	status_t status = sigwait(&set, &signal);
+	status_t status = sigwait_internal(&set, &signal);
-	if (status < B_OK)
+	if (status == B_INTERRUPTED) {
-		return syscall_restart_handle_post(status);
+		// make sure we'll be restarted
+		struct thread* thread = thread_get_current_thread();
+		atomic_or(&thread->flags,
+			THREAD_FLAGS_ALWAYS_RESTART_SYSCALL | THREAD_FLAGS_RESTART_SYSCALL);
+		return status;
+	}
 
 	return user_memcpy(_userSignal, &signal, sizeof(int));
 }
@@ -1027,7 +1072,7 @@ _user_sigsuspend(const sigset_t *userMask)
 	if (user_memcpy(&mask, userMask, sizeof(sigset_t)) < B_OK)
 		return B_BAD_ADDRESS;
 
-	return sigsuspend(&mask);
+	return sigsuspend_internal(&mask);
 }
 
 
@@ -1042,7 +1087,7 @@ _user_sigpending(sigset_t *userSet)
 	if (!IS_USER_ADDRESS(userSet))
 		return B_BAD_ADDRESS;
 
-	status = sigpending(&set);
+	status = sigpending_internal(&set);
 	if (status == B_OK
 		&& user_memcpy(userSet, &set, sizeof(sigset_t)) < B_OK)
 		return B_BAD_ADDRESS;