kernel: Protect signal data with Team::signal_lock

headers/private/kernel/thread_types.h

@@ -291,6 +291,8 @@ struct Team : TeamThreadIteratorEntry<team_id>, KernelReferenceable,
 		bool		initialized;	// true when the state has been initialized
 	} exit;
 
+	spinlock		signal_lock;
+
 public:
 								~Team();
 
@@ -398,7 +400,7 @@ private:
 
 			BKernel::QueuedSignalsCounter* fQueuedSignalsCounter;
 			BKernel::PendingSignals	fPendingSignals;
-									// protected by scheduler lock
+									// protected by signal_lock
 			struct sigaction 	fSignalActions[MAX_SIGNAL_NUMBER];
 									// indexed signal - 1, protected by fLock
 
@@ -429,7 +431,7 @@ struct Thread : TeamThreadIteratorEntry<thread_id>, KernelReferenceable,
 	int32			pinned_to_cpu;	// only accessed by this thread or in the
 									// scheduler, when thread is not running
 
-	sigset_t		sig_block_mask;	// protected by scheduler lock,
+	sigset_t		sig_block_mask;	// protected by team->signal_lock,
 									// only modified by the thread itself
 	sigset_t		sigsuspend_original_unblocked_mask;
 		// non-0 after a return from _user_sigsuspend(), containing the inverted
@@ -481,7 +483,8 @@ struct Thread : TeamThreadIteratorEntry<thread_id>, KernelReferenceable,
 		/* this field may only stay in debug builds in the future */
 
 	BKernel::Team	*team;	// protected by team lock, thread lock, scheduler
-							// lock
+							// lock, team_lock
+	spinlock		team_lock;
 
 	struct {
 		sem_id		sem;		// immutable after thread creation
@@ -604,7 +607,7 @@ private:
 			mutex				fLock;
 
 			BKernel::PendingSignals	fPendingSignals;
-									// protected by scheduler lock
+									// protected by team->signal_lock
 
 			UserTimerList		fUserTimers;			// protected by fLock
 			ThreadTimeUserTimerList fCPUTimeUserTimers;

src/system/kernel/UserEvent.cpp

@@ -115,7 +115,7 @@ TeamSignalEvent::Fire()
 	fSignal->AcquireReference();
 		// one reference is transferred to send_signal_to_team_locked
 
-	InterruptsSpinLocker locker(gSchedulerLock);
+	InterruptsSpinLocker locker(fTeam->signal_lock);
 	status_t error = send_signal_to_team_locked(fTeam, fSignal->Number(),
 		fSignal, B_DO_NOT_RESCHEDULE);
 	locker.Unlock();
@@ -169,11 +169,12 @@ ThreadSignalEvent::Fire()
 
 	fSignal->AcquireReference();
 		// one reference is transferred to send_signal_to_team_locked
-
+	InterruptsSpinLocker teamLocker(fThread->team_lock);
-	InterruptsSpinLocker locker(gSchedulerLock);
+	SpinLocker locker(fThread->team->signal_lock);
 	status_t error = send_signal_to_thread_locked(fThread, fSignal->Number(),
 		fSignal, B_DO_NOT_RESCHEDULE);
 	locker.Unlock();
+	teamLocker.Unlock();
 
 	// There are situations (for certain signals), in which
 	// send_signal_to_team_locked() succeeds without queuing the signal.

src/system/kernel/signal.cpp

@@ -746,7 +746,7 @@ class SigSuspendDone : public AbstractTraceEntry {
 
 /*!	Updates the given thread's Thread::flags field according to what signals are
 	pending.
-	The caller must hold the scheduler lock.
+	The caller must hold \c team->signal_lock.
 */
 static void
 update_thread_signals_flag(Thread* thread)
@@ -761,7 +761,7 @@ update_thread_signals_flag(Thread* thread)
 
 /*!	Updates the current thread's Thread::flags field according to what signals
 	are pending.
-	The caller must hold the scheduler lock.
+	The caller must hold \c team->signal_lock.
 */
 static void
 update_current_thread_signals_flag()
@@ -772,7 +772,7 @@ update_current_thread_signals_flag()
 
 /*!	Updates all of the given team's threads' Thread::flags fields according to
 	what signals are pending.
-	The caller must hold the scheduler lock.
+	The caller must hold \c signal_lock.
 */
 static void
 update_team_threads_signal_flag(Team* team)
@@ -824,7 +824,7 @@ notify_debugger(Thread* thread, Signal* signal, struct sigaction& handler,
 	After dequeuing the signal the Thread::flags field of the affected threads
 	are updated.
 	The caller gets a reference to the returned signal, if any.
-	The caller must hold the scheduler lock.
+	The caller must hold \c team->signal_lock.
 	\param thread The thread.
 	\param nonBlocked The mask of non-blocked signals.
 	\param buffer If the signal is not queued this buffer is returned. In this
@@ -916,7 +916,7 @@ handle_signals(Thread* thread)
 	Team* team = thread->team;
 
 	TeamLocker teamLocker(team);
-	InterruptsSpinLocker schedulerLocker(gSchedulerLock);
+	InterruptsSpinLocker locker(thread->team->signal_lock);
 
 	// If userland requested to defer signals, we check now, if this is
 	// possible.
@@ -947,7 +947,7 @@ handle_signals(Thread* thread)
 			initialIteration = false;
 		} else {
 			teamLocker.Lock();
-			schedulerLocker.Lock();
+			locker.Lock();
 
 			signalMask = thread->AllPendingSignals() & nonBlockedMask;
 		}
@@ -957,7 +957,7 @@ handle_signals(Thread* thread)
 		if ((signalMask & KILL_SIGNALS) == 0
 			&& (atomic_get(&thread->debug_info.flags) & B_THREAD_DEBUG_STOP)
 				!= 0) {
-			schedulerLocker.Unlock();
+			locker.Unlock();
 			teamLocker.Unlock();
 
 			user_debug_stop_thread();
@@ -976,7 +976,7 @@ handle_signals(Thread* thread)
 		ASSERT(signal != NULL);
 		SignalHandledCaller signalHandledCaller(signal);
 
-		schedulerLocker.Unlock();
+		locker.Unlock();
 
 		// get the action for the signal
 		struct sigaction handler;
@@ -1122,13 +1122,16 @@ handle_signals(Thread* thread)
 
 					// Suspend the thread, unless there's already a signal to
 					// continue or kill pending.
-					InterruptsSpinLocker schedulerLocker(gSchedulerLock);
+					locker.Lock();
-					if ((thread->AllPendingSignals()
+					bool resume = (thread->AllPendingSignals()
-							& (CONTINUE_SIGNALS | KILL_SIGNALS)) == 0) {
+								& (CONTINUE_SIGNALS | KILL_SIGNALS)) != 0;
+					locker.Unlock();
+
+					if (!resume) {
+						InterruptsSpinLocker schedulerLocker(gSchedulerLock);
 						thread->next_state = B_THREAD_SUSPENDED;
 						scheduler_reschedule();
 					}
-					schedulerLocker.Unlock();
 
 					continue;
 				}
@@ -1215,7 +1218,7 @@ handle_signals(Thread* thread)
 		TRACE(("### Setting up custom signal handler frame...\n"));
 
 		// save the old block mask -- we may need to adjust it for the handler
-		schedulerLocker.Lock();
+		locker.Lock();
 
 		sigset_t oldBlockMask = thread->sigsuspend_original_unblocked_mask != 0
 			? ~thread->sigsuspend_original_unblocked_mask
@@ -1232,7 +1235,7 @@ handle_signals(Thread* thread)
 
 		update_current_thread_signals_flag();
 
-		schedulerLocker.Unlock();
+		locker.Unlock();
 
 		setup_signal_frame(thread, &handler, signal, oldBlockMask);
 
@@ -1263,7 +1266,7 @@ handle_signals(Thread* thread)
 
 /*!	Checks whether the given signal is blocked for the given team (i.e. all of
 	its threads).
-	The caller must hold the team's lock and the scheduler lock.
+	The caller must hold the team's lock and \c signal_lock.
 */
 bool
 is_team_signal_blocked(Team* team, int signal)
@@ -1308,7 +1311,7 @@ signal_get_user_stack(addr_t address, stack_t* stack)
 
 
 /*!	Checks whether any non-blocked signal is pending for the current thread.
-	The caller must hold the scheduler lock.
+	The caller must hold \c team->signal_lock.
 	\param thread The current thread.
 */
 static bool
@@ -1338,7 +1341,7 @@ has_permission_to_signal(Signal* signal, Team* team)
 /*!	Delivers a signal to the \a thread, but doesn't handle the signal -- it just
 	makes sure the thread gets the signal, i.e. unblocks it if needed.
 
-	The caller must hold the scheduler lock.
+	The caller must hold \c team->signal_lock.
 
 	\param thread The thread the signal shall be delivered to.
 	\param signalNumber The number of the signal to be delivered. If \c 0, no
@@ -1374,6 +1377,7 @@ send_signal_to_thread_locked(Thread* thread, uint32 signalNumber,
 
 	if (thread->team == team_get_kernel_team()) {
 		// Signals to kernel threads will only wake them up
+		SpinLocker _(gSchedulerLock);
 		if (thread->state == B_THREAD_SUSPENDED)
 			scheduler_enqueue_in_run_queue(thread);
 		return B_OK;
@@ -1397,10 +1401,12 @@ send_signal_to_thread_locked(Thread* thread, uint32 signalNumber,
 				mainThread->AddPendingSignal(SIGKILLTHR);
 
 				// wake up main thread
+				SpinLocker locker(gSchedulerLock);
 				if (mainThread->state == B_THREAD_SUSPENDED)
 					scheduler_enqueue_in_run_queue(mainThread);
 				else
 					thread_interrupt(mainThread, true);
+				locker.Unlock();
 
 				update_thread_signals_flag(mainThread);
 			}
@@ -1408,24 +1414,31 @@ send_signal_to_thread_locked(Thread* thread, uint32 signalNumber,
 			// supposed to fall through
 		}
 		case SIGKILLTHR:
+		{
 			// Wake up suspended threads and interrupt waiting ones
+			SpinLocker locker(gSchedulerLock);
 			if (thread->state == B_THREAD_SUSPENDED)
 				scheduler_enqueue_in_run_queue(thread);
 			else
 				thread_interrupt(thread, true);
-			break;
 
+			break;
+		}
 		case SIGNAL_CONTINUE_THREAD:
+		{
 			// wake up thread, and interrupt its current syscall
+			SpinLocker locker(gSchedulerLock);
 			if (thread->state == B_THREAD_SUSPENDED)
 				scheduler_enqueue_in_run_queue(thread);
 
 			atomic_or(&thread->flags, THREAD_FLAGS_DONT_RESTART_SYSCALL);
 			break;
-
+		}
 		case SIGCONT:
+		{
 			// Wake up thread if it was suspended, otherwise interrupt it, if
 			// the signal isn't blocked.
+			SpinLocker locker(gSchedulerLock);
 			if (thread->state == B_THREAD_SUSPENDED)
 				scheduler_enqueue_in_run_queue(thread);
 			else if ((SIGNAL_TO_MASK(SIGCONT) & ~thread->sig_block_mask) != 0)
@@ -1434,7 +1447,7 @@ send_signal_to_thread_locked(Thread* thread, uint32 signalNumber,
 			// remove any pending stop signals
 			thread->RemovePendingSignals(STOP_SIGNALS);
 			break;
-
+		}
 		default:
 			// If the signal is not masked, interrupt the thread, if it is
 			// currently waiting (interruptibly).
@@ -1442,6 +1455,7 @@ send_signal_to_thread_locked(Thread* thread, uint32 signalNumber,
 						& (~thread->sig_block_mask | SIGNAL_TO_MASK(SIGCHLD)))
 					!= 0) {
 				// Interrupt thread if it was waiting
+				SpinLocker locker(gSchedulerLock);
 				thread_interrupt(thread, false);
 			}
 			break;
@@ -1455,8 +1469,6 @@ send_signal_to_thread_locked(Thread* thread, uint32 signalNumber,
 
 /*!	Sends the given signal to the given thread.
 
-	The caller must not hold the scheduler lock.
-
 	\param thread The thread the signal shall be sent to.
 	\param signal The signal to be delivered. If the signal's number is \c 0, no
 		actual signal will be delivered. Only delivery checks will be performed.
@@ -1482,15 +1494,19 @@ send_signal_to_thread(Thread* thread, const Signal& signal, uint32 flags)
 	if (error != B_OK)
 		return error;
 
-	InterruptsSpinLocker schedulerLocker(gSchedulerLock);
+	InterruptsSpinLocker teamLocker(thread->team_lock);
+	SpinLocker locker(thread->team->signal_lock);
 
 	error = send_signal_to_thread_locked(thread, signal.Number(), signalToQueue,
 		flags);
 	if (error != B_OK)
 		return error;
 
+	locker.Unlock();
+	teamLocker.Unlock();
+
 	if ((flags & B_DO_NOT_RESCHEDULE) == 0)
-		scheduler_reschedule_if_necessary_locked();
+		scheduler_reschedule_if_necessary();
 
 	return B_OK;
 }
@@ -1498,8 +1514,6 @@ send_signal_to_thread(Thread* thread, const Signal& signal, uint32 flags)
 
 /*!	Sends the given signal to the thread with the given ID.
 
-	The caller must not hold the scheduler lock.
-
 	\param threadID The ID of the thread the signal shall be sent to.
 	\param signal The signal to be delivered. If the signal's number is \c 0, no
 		actual signal will be delivered. Only delivery checks will be performed.
@@ -1528,7 +1542,7 @@ send_signal_to_thread_id(thread_id threadID, const Signal& signal, uint32 flags)
 
 /*!	Sends the given signal to the given team.
 
-	The caller must hold the scheduler lock.
+	The caller must hold \c signal_lock.
 
 	\param team The team the signal shall be sent to.
 	\param signalNumber The number of the signal to be delivered. If \c 0, no
@@ -1591,6 +1605,7 @@ send_signal_to_team_locked(Team* team, uint32 signalNumber, Signal* signal,
 				mainThread->AddPendingSignal(SIGKILLTHR);
 
 				// wake up main thread
+				SpinLocker _(gSchedulerLock);
 				if (mainThread->state == B_THREAD_SUSPENDED)
 					scheduler_enqueue_in_run_queue(mainThread);
 				else
@@ -1604,6 +1619,7 @@ send_signal_to_team_locked(Team* team, uint32 signalNumber, Signal* signal,
 			// don't block the signal.
 			for (Thread* thread = team->thread_list; thread != NULL;
 					thread = thread->team_next) {
+				SpinLocker _(gSchedulerLock);
 				if (thread->state == B_THREAD_SUSPENDED) {
 					scheduler_enqueue_in_run_queue(thread);
 				} else if ((SIGNAL_TO_MASK(SIGCONT) & ~thread->sig_block_mask)
@@ -1645,17 +1661,16 @@ send_signal_to_team_locked(Team* team, uint32 signalNumber, Signal* signal,
 					thread = thread->team_next) {
 				sigset_t nonBlocked = ~thread->sig_block_mask
 					| SIGNAL_TO_MASK(SIGCHLD);
-				if ((thread->AllPendingSignals() & nonBlocked) != 0)
+				if ((thread->AllPendingSignals() & nonBlocked) != 0) {
+					SpinLocker _(gSchedulerLock);
 					thread_interrupt(thread, false);
+				}
 			}
 			break;
 	}
 
 	update_team_threads_signal_flag(team);
 
-	if ((flags & B_DO_NOT_RESCHEDULE) == 0)
-		scheduler_reschedule_if_necessary_locked();
-
 	return B_OK;
 }
 
@@ -1687,10 +1702,17 @@ send_signal_to_team(Team* team, const Signal& signal, uint32 flags)
 	if (error != B_OK)
 		return error;
 
-	InterruptsSpinLocker schedulerLocker(gSchedulerLock);
+	InterruptsSpinLocker locker(team->signal_lock);
 
-	return send_signal_to_team_locked(team, signal.Number(), signalToQueue,
+	error = send_signal_to_team_locked(team, signal.Number(), signalToQueue,
-		flags);
+			flags);
+
+	locker.Unlock();
+
+	if ((flags & B_DO_NOT_RESCHEDULE) == 0)
+		scheduler_reschedule_if_necessary();
+
+	return error;
 }
 
 
@@ -1876,7 +1898,7 @@ sigprocmask_internal(int how, const sigset_t* set, sigset_t* oldSet)
 {
 	Thread* thread = thread_get_current_thread();
 
-	InterruptsSpinLocker schedulerLocker(gSchedulerLock);
+	InterruptsSpinLocker _(thread->team->signal_lock);
 
 	sigset_t oldMask = thread->sig_block_mask;
 
@@ -1947,7 +1969,7 @@ sigaction_internal(int signal, const struct sigaction* act,
 	if ((act && act->sa_handler == SIG_IGN)
 		|| (act && act->sa_handler == SIG_DFL
 			&& (SIGNAL_TO_MASK(signal) & DEFAULT_IGNORE_SIGNALS) != 0)) {
-		InterruptsSpinLocker schedulerLocker(gSchedulerLock);
+		InterruptsSpinLocker locker(team->signal_lock);
 
 		team->RemovePendingSignal(signal);
 
@@ -1989,7 +2011,7 @@ sigwait_internal(const sigset_t* set, siginfo_t* info, uint32 flags,
 
 	Thread* thread = thread_get_current_thread();
 
-	InterruptsSpinLocker schedulerLocker(gSchedulerLock);
+	InterruptsSpinLocker locker(thread->team->signal_lock);
 
 	bool timedOut = false;
 	status_t error = B_OK;
@@ -2009,7 +2031,7 @@ sigwait_internal(const sigset_t* set, siginfo_t* info, uint32 flags,
 			ASSERT(signal != NULL);
 
 			SignalHandledCaller signalHandledCaller(signal);
-			schedulerLocker.Unlock();
+			locker.Unlock();
 
 			info->si_signo = signal->Number();
 			info->si_code = signal->SignalCode();
@@ -2041,7 +2063,7 @@ sigwait_internal(const sigset_t* set, siginfo_t* info, uint32 flags,
 			thread_prepare_to_block(thread, flags, THREAD_BLOCK_TYPE_SIGNAL,
 				NULL);
 
-			schedulerLocker.Unlock();
+			locker.Unlock();
 
 			if ((flags & B_ABSOLUTE_TIMEOUT) != 0) {
 				error = thread_block_with_timeout(flags, timeout);
@@ -2050,13 +2072,13 @@ sigwait_internal(const sigset_t* set, siginfo_t* info, uint32 flags,
 						// POSIX requires EAGAIN (B_WOULD_BLOCK) on timeout
 					timedOut = true;
 
-					schedulerLocker.Lock();
+					locker.Lock();
 					break;
 				}
 			} else
 				thread_block();
 
-			schedulerLocker.Lock();
+			locker.Lock();
 		}
 
 		// restore the original block mask
@@ -2082,7 +2104,7 @@ sigsuspend_internal(const sigset_t* _mask)
 
 	Thread* thread = thread_get_current_thread();
 
-	InterruptsSpinLocker schedulerLocker(gSchedulerLock);
+	InterruptsSpinLocker locker(thread->team->signal_lock);
 
 	// Set the new block mask and block until interrupted. We might be here
 	// after a syscall restart, in which case sigsuspend_original_unblocked_mask
@@ -2096,7 +2118,10 @@ sigsuspend_internal(const sigset_t* _mask)
 	while (!has_signals_pending(thread)) {
 		thread_prepare_to_block(thread, B_CAN_INTERRUPT,
 			THREAD_BLOCK_TYPE_SIGNAL, NULL);
-		thread_block_locked(thread);
+
+		locker.Unlock();
+		thread_block();
+		locker.Lock();
 	}
 
 	// Set sigsuspend_original_unblocked_mask (guaranteed to be non-0 due to
@@ -2121,7 +2146,7 @@ sigpending_internal(sigset_t* set)
 	if (set == NULL)
 		return B_BAD_VALUE;
 
-	InterruptsSpinLocker schedulerLocker(gSchedulerLock);
+	InterruptsSpinLocker locker(thread->team->signal_lock);
 
 	*set = thread->AllPendingSignals() & thread->sig_block_mask;
 
@@ -2412,13 +2437,13 @@ _user_restore_signal_frame(struct signal_frame_data* userSignalFrameData)
 	}
 
 	// restore the signal block mask
-	InterruptsSpinLocker schedulerLocker(gSchedulerLock);
+	InterruptsSpinLocker locker(thread->team->signal_lock);
 
 	thread->sig_block_mask
 		= signalFrameData.context.uc_sigmask & BLOCKABLE_SIGNALS;
 	update_current_thread_signals_flag();
 
-	schedulerLocker.Unlock();
+	locker.Unlock();
 
 	// restore the syscall restart related thread flags and the syscall restart
 	// parameters

src/system/kernel/team.cpp

@@ -490,6 +490,8 @@ Team::Team(team_id id, bool kernel)
 	// init dead/stopped/continued children condition vars
 	dead_children.condition_variable.Init(&dead_children, "team children");
 
+	B_INITIALIZE_SPINLOCK(&signal_lock);
+
 	fQueuedSignalsCounter = new(std::nothrow) BKernel::QueuedSignalsCounter(
 		kernel ? -1 : MAX_QUEUED_SIGNALS);
 	memset(fSignalActions, 0, sizeof(fSignalActions));

src/system/kernel/thread.cpp

@@ -206,6 +206,7 @@ Thread::Thread(const char* name, thread_id threadID, struct cpu_ent* cpu)
 	mutex_init_etc(&fLock, lockName, MUTEX_FLAG_CLONE_NAME);
 
 	B_INITIALIZE_SPINLOCK(&time_lock);
+	B_INITIALIZE_SPINLOCK(&team_lock);
 
 	// init name
 	if (name != NULL)
@@ -1103,11 +1104,13 @@ undertaker(void* /*args*/)
 		Team* kernelTeam = team_get_kernel_team();
 		TeamLocker kernelTeamLocker(kernelTeam);
 		thread->Lock();
-		InterruptsSpinLocker schedulerLocker(gSchedulerLock);
+		InterruptsSpinLocker signalLocker(kernelTeam->signal_lock);
+		SpinLocker schedulerLocker(gSchedulerLock);
 
 		remove_thread_from_team(kernelTeam, thread);
 
 		schedulerLocker.Unlock();
+		signalLocker.Unlock();
 		kernelTeamLocker.Unlock();
 
 		// free the thread structure
@@ -1986,6 +1989,7 @@ thread_exit(void)
 		// swap address spaces, to make sure we're running on the kernel's pgdir
 		vm_swap_address_space(team->address_space, VMAddressSpace::Kernel());
 
+		SpinLocker teamLocker(thread->team_lock);
 		SpinLocker schedulerLocker(gSchedulerLock);
 			// removing the thread and putting its death entry to the parent
 			// team needs to be an atomic operation
@@ -2014,6 +2018,8 @@ thread_exit(void)
 		remove_thread_from_team(team, thread);
 		insert_thread_into_team(kernelTeam, thread);
 
+		teamLocker.Unlock();
+
 		if (team->death_entry != NULL) {
 			if (--team->death_entry->remaining_threads == 0)
 				team->death_entry->condition.NotifyOne(true, B_OK);
@@ -3282,7 +3288,8 @@ _user_cancel_thread(thread_id threadID, void (*cancelFunction)(int))
 	thread->cancel_function = cancelFunction;
 
 	// send the cancellation signal to the thread
-	InterruptsSpinLocker schedulerLocker(gSchedulerLock);
+	InterruptsSpinLocker teamLocker(thread->team_lock);
+	SpinLocker locker(thread->team->signal_lock);
 	return send_signal_to_thread_locked(thread, SIGNAL_CANCEL_THREAD, NULL, 0);
 }