Added team_debug_info::debugger_changed_condition to serialize changes to the

installed team debugger and adjusted the code accordingly. It's not needed yet,
but I intend to add support for software breakpoints and those require a bit of
uninitialization that needs to be synchronized with debugger changes and can't
be done with interrupts disabled.


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

headers/private/kernel/user_debugger.h

@@ -20,6 +20,7 @@
 #define	B_DEBUG_PROFILE_BUFFER_FLUSH_THRESHOLD	70			/* in % */
 
 
+struct ConditionVariable;
 struct function_profile_info;
 struct thread;
 
@@ -61,6 +62,14 @@ struct team_debug_info {
 	vint32		image_event;
 		// counter incremented whenever an image is created/deleted
 
+	struct ConditionVariable* debugger_changed_condition;
+		// Set whenever someone is going (or planning) to change the debugger.
+		// If one wants to do the same, one has to wait for this condition.
+		// Both threads lock (outer) and team debug info lock (inner) have to
+		// be held when accessing this field. After setting to a condition
+		// variable the thread won't be deleted (until unsetting it) -- it might
+		// be removed from the team hash table, though.
+
 	struct arch_team_debug_info	arch_info;
 };
 

src/system/kernel/debug/user_debugger.cpp

@@ -223,8 +223,10 @@ clear_team_debug_info(struct team_debug_info *info, bool initLock)
 		info->causing_thread = -1;
 		info->image_event = 0;
 
-		if (initLock)
+		if (initLock) {
 			B_INITIALIZE_SPINLOCK(&info->lock);
+			info->debugger_changed_condition = NULL;
+		}
 	}
 }
 
@@ -348,6 +350,90 @@ destroy_thread_debug_info(struct thread_debug_info *info)
 }
 
 
+static status_t
+prepare_debugger_change(team_id teamID, ConditionVariable& condition,
+	struct team*& team)
+{
+	// We look up the team by ID, even in case of the current team, so we can be
+	// sure, that the team is not already dying.
+	if (teamID == B_CURRENT_TEAM)
+		teamID = thread_get_current_thread()->team->id;
+
+	while (true) {
+		// get the team
+		InterruptsSpinLocker teamLocker(gTeamSpinlock);
+
+		team = team_get_team_struct_locked(teamID);
+		if (team == NULL)
+			return B_BAD_TEAM_ID;
+
+		// don't allow messing with the kernel team
+		if (team == team_get_kernel_team())
+			return B_NOT_ALLOWED;
+
+		// check whether the condition is already set
+		SpinLocker threadLocker(gThreadSpinlock);
+		SpinLocker debugInfoLocker(team->debug_info.lock);
+
+		if (team->debug_info.debugger_changed_condition == NULL) {
+			// nobody there yet -- set our condition variable and be done
+			team->debug_info.debugger_changed_condition = &condition;
+			return B_OK;
+		}
+
+		// we'll have to wait
+		ConditionVariableEntry entry;
+		team->debug_info.debugger_changed_condition->Add(&entry);
+
+		debugInfoLocker.Unlock();
+		threadLocker.Unlock();
+		teamLocker.Unlock();
+
+		entry.Wait();
+	}
+}
+
+
+static void
+prepare_debugger_change(struct team* team, ConditionVariable& condition)
+{
+	while (true) {
+		// check whether the condition is already set
+		InterruptsSpinLocker threadLocker(gThreadSpinlock);
+		SpinLocker debugInfoLocker(team->debug_info.lock);
+
+		if (team->debug_info.debugger_changed_condition == NULL) {
+			// nobody there yet -- set our condition variable and be done
+			team->debug_info.debugger_changed_condition = &condition;
+			return;
+		}
+
+		// we'll have to wait
+		ConditionVariableEntry entry;
+		team->debug_info.debugger_changed_condition->Add(&entry);
+
+		debugInfoLocker.Unlock();
+		threadLocker.Unlock();
+
+		entry.Wait();
+	}
+}
+
+
+static void
+finish_debugger_change(struct team* team)
+{
+	// unset our condition variable and notify all threads waiting on it
+	InterruptsSpinLocker threadLocker(gThreadSpinlock);
+	SpinLocker debugInfoLocker(team->debug_info.lock);
+
+	ConditionVariable* condition = team->debug_info.debugger_changed_condition;
+	team->debug_info.debugger_changed_condition = NULL;
+
+	condition->NotifyAll(true);
+}
+
+
 void
 user_debug_prepare_for_exec()
 {
@@ -1406,11 +1492,13 @@ nub_thread_cleanup(struct thread *nubThread)
 	TRACE(("nub_thread_cleanup(%ld): debugger port: %ld\n", nubThread->id,
 		nubThread->team->debug_info.debugger_port));
 
+	ConditionVariable debugChangeCondition;
+	prepare_debugger_change(nubThread->team, debugChangeCondition);
+
 	team_debug_info teamDebugInfo;
 	bool destroyDebugInfo = false;
 
 	cpu_status state = disable_interrupts();
-	GRAB_TEAM_LOCK();
 	GRAB_TEAM_DEBUG_INFO_LOCK(nubThread->team->debug_info);
 
 	team_debug_info &info = nubThread->team->debug_info;
@@ -1425,9 +1513,10 @@ nub_thread_cleanup(struct thread *nubThread)
 	update_threads_debugger_installed_flag(nubThread->team);
 
 	RELEASE_TEAM_DEBUG_INFO_LOCK(nubThread->team->debug_info);
-	RELEASE_TEAM_LOCK();
 	restore_interrupts(state);
 
+	finish_debugger_change(nubThread->team);
+
 	if (destroyDebugInfo)
 		destroy_team_debug_info(&teamDebugInfo);
 
@@ -2526,14 +2615,24 @@ install_team_debugger(team_id teamID, port_id debuggerPort,
 	}
 	team_id debuggerTeam = debuggerPortInfo.team;
 
-	// check the debugger team: It must neither be the kernel team nor the
+	// Check the debugger team: It must neither be the kernel team nor the
-	// debugged team
+	// debugged team.
 	if (debuggerTeam == team_get_kernel_team_id() || debuggerTeam == teamID) {
 		TRACE(("install_team_debugger(): Can't debug kernel or debugger team. "
 			"debugger: %ld, debugged: %ld\n", debuggerTeam, teamID));
 		return B_NOT_ALLOWED;
 	}
 
+	// get the team
+	struct team* team;
+	ConditionVariable debugChangeCondition;
+	error = prepare_debugger_change(teamID, debugChangeCondition, team);
+	if (error != B_OK)
+		return error;
+
+	// get the real team ID
+	teamID = team->id;
+
 	// check, if a debugger is already installed
 
 	bool done = false;
@@ -2544,90 +2643,73 @@ install_team_debugger(team_id teamID, port_id debuggerPort,
 	port_id nubPort = -1;
 
 	cpu_status state = disable_interrupts();
-	GRAB_TEAM_LOCK();
+	GRAB_TEAM_DEBUG_INFO_LOCK(team->debug_info);
 
-	// get a real team ID
+	int32 teamDebugFlags = team->debug_info.flags;
-	// We look up the team by ID, even in case of the current team, so we can be
-	// sure, that the team is not already dying.
-	if (teamID == B_CURRENT_TEAM)
-		teamID = thread_get_current_thread()->team->id;
 
-	struct team *team = team_get_team_struct_locked(teamID);
+	if (teamDebugFlags & B_TEAM_DEBUG_DEBUGGER_INSTALLED) {
-
+		// There's already a debugger installed.
-	if (team) {
+		if (teamDebugFlags & B_TEAM_DEBUG_DEBUGGER_HANDOVER) {
-		GRAB_TEAM_DEBUG_INFO_LOCK(team->debug_info);
+			if (dontReplace) {
-
+				// We're fine with already having a debugger.
-		int32 teamDebugFlags = team->debug_info.flags;
+				error = B_OK;
-
+				done = true;
-		if (team == team_get_kernel_team()) {
+				result = team->debug_info.nub_port;
-			// don't allow to debug the kernel
+			} else if (
-			error = B_NOT_ALLOWED;
+				teamDebugFlags & B_TEAM_DEBUG_DEBUGGER_HANDING_OVER) {
-		} else if (teamDebugFlags & B_TEAM_DEBUG_DEBUGGER_INSTALLED) {
+				// Another debugger is in the process of installing itself
-			// There's already a debugger installed.
+				// as the team's debugger.
-			if (teamDebugFlags & B_TEAM_DEBUG_DEBUGGER_HANDOVER) {
-				if (dontReplace) {
-					// We're fine with already having a debugger.
-					error = B_OK;
-					done = true;
-					result = team->debug_info.nub_port;
-				} else if (
-					teamDebugFlags & B_TEAM_DEBUG_DEBUGGER_HANDING_OVER) {
-					// Another debugger is in the process of installing itself
-					// as the team's debugger.
-					error = (dontReplace ? B_OK : B_BAD_VALUE);
-					done = true;
-					result = team->debug_info.nub_port;
-				} else {
-					// a handover to another debugger is requested
-					// Set the handing-over flag -- we'll clear both flags after
-					// having sent the handed-over message to the new debugger.
-					atomic_or(&team->debug_info.flags,
-						B_TEAM_DEBUG_DEBUGGER_HANDING_OVER);
-
-					oldDebuggerPort = team->debug_info.debugger_port;
-					result = nubPort = team->debug_info.nub_port;
-					if (causingThread < 0)
-						causingThread = team->debug_info.causing_thread;
-
-					// set the new debugger
-					install_team_debugger_init_debug_infos(team, debuggerTeam,
-						debuggerPort, nubPort, team->debug_info.nub_thread,
-						team->debug_info.debugger_write_lock, causingThread);
-
-					releaseDebugInfoLock = false;
-					handOver = true;
-					done = true;
-
-					// finally set the new port owner
-					if (set_port_owner(nubPort, debuggerTeam) != B_OK) {
-						// The old debugger must just have died. Just proceed as
-						// if there was no debugger installed. We may still be too
-						// early, in which case we'll fail, but this race condition
-						// should be unbelievably rare and relatively harmless.
-						handOver = false;
-						done = false;
-					}
-				}
-			} else {
-				// there's already a debugger installed
 				error = (dontReplace ? B_OK : B_BAD_VALUE);
 				done = true;
 				result = team->debug_info.nub_port;
+			} else {
+				// a handover to another debugger is requested
+				// Set the handing-over flag -- we'll clear both flags after
+				// having sent the handed-over message to the new debugger.
+				atomic_or(&team->debug_info.flags,
+					B_TEAM_DEBUG_DEBUGGER_HANDING_OVER);
+
+				oldDebuggerPort = team->debug_info.debugger_port;
+				result = nubPort = team->debug_info.nub_port;
+				if (causingThread < 0)
+					causingThread = team->debug_info.causing_thread;
+
+				// set the new debugger
+				install_team_debugger_init_debug_infos(team, debuggerTeam,
+					debuggerPort, nubPort, team->debug_info.nub_thread,
+					team->debug_info.debugger_write_lock, causingThread);
+
+				releaseDebugInfoLock = false;
+				handOver = true;
+				done = true;
+
+				// finally set the new port owner
+				if (set_port_owner(nubPort, debuggerTeam) != B_OK) {
+					// The old debugger must just have died. Just proceed as
+					// if there was no debugger installed. We may still be too
+					// early, in which case we'll fail, but this race condition
+					// should be unbelievably rare and relatively harmless.
+					handOver = false;
+					done = false;
+				}
 			}
-		} else if ((teamDebugFlags & B_TEAM_DEBUG_DEBUGGER_DISABLED) != 0
+		} else {
-			&& useDefault) {
+			// there's already a debugger installed
-			// No debugger yet, disable_debugger() had been invoked, and we
+			error = (dontReplace ? B_OK : B_BAD_VALUE);
-			// would install the default debugger. Just fail.
+			done = true;
-			error = B_BAD_VALUE;
+			result = team->debug_info.nub_port;
 		}
+	} else if ((teamDebugFlags & B_TEAM_DEBUG_DEBUGGER_DISABLED) != 0
+		&& useDefault) {
+		// No debugger yet, disable_debugger() had been invoked, and we
+		// would install the default debugger. Just fail.
+		error = B_BAD_VALUE;
+	}
 
-		// in case of a handover the lock has already been released
+	// in case of a handover the lock has already been released
-		if (releaseDebugInfoLock)
+	if (releaseDebugInfoLock)
-			RELEASE_TEAM_DEBUG_INFO_LOCK(team->debug_info);
+		RELEASE_TEAM_DEBUG_INFO_LOCK(team->debug_info);
-	} else
-		error = B_BAD_TEAM_ID;
 
-	RELEASE_TEAM_LOCK();
 	restore_interrupts(state);
 
 	if (handOver) {
@@ -2650,31 +2732,25 @@ install_team_debugger(team_id teamID, port_id debuggerPort,
 		}
 
 		// clear the handed-over and handing-over flags
-		cpu_status state = disable_interrupts();
+		state = disable_interrupts();
-		GRAB_TEAM_LOCK();
+		GRAB_TEAM_DEBUG_INFO_LOCK(team->debug_info);
 
-		team = team_get_team_struct_locked(teamID);
+		int32 teamDebugFlags = team->debug_info.flags;
 
-		if (team) {
+		if ((teamDebugFlags & B_TEAM_DEBUG_DEBUGGER_INSTALLED) != 0
-			GRAB_TEAM_DEBUG_INFO_LOCK(team->debug_info);
+			&& (teamDebugFlags & B_TEAM_DEBUG_DEBUGGER_HANDING_OVER) != 0
-
+			&& team->debug_info.debugger_port == debuggerPort) {
-			int32 teamDebugFlags = team->debug_info.flags;
+			// Everything is as we left it above, so just clear the flags.
-
+			atomic_and(&team->debug_info.flags,
-			if ((teamDebugFlags & B_TEAM_DEBUG_DEBUGGER_INSTALLED) != 0
+				~(B_TEAM_DEBUG_DEBUGGER_HANDOVER
-				&& (teamDebugFlags & B_TEAM_DEBUG_DEBUGGER_HANDING_OVER) != 0
+					| B_TEAM_DEBUG_DEBUGGER_HANDING_OVER));
-				&& team->debug_info.debugger_port == debuggerPort) {
-				// Everything is as we left it above, so just clear the flags.
-				atomic_and(&team->debug_info.flags,
-					~(B_TEAM_DEBUG_DEBUGGER_HANDOVER
-						| B_TEAM_DEBUG_DEBUGGER_HANDING_OVER));
-			}
-
-			RELEASE_TEAM_DEBUG_INFO_LOCK(team->debug_info);
 		}
 
-		RELEASE_TEAM_LOCK();
+		RELEASE_TEAM_DEBUG_INFO_LOCK(team->debug_info);
 		restore_interrupts(state);
 
+		finish_debugger_change(team);
+
 		// notify the nub thread
 		kill_interruptable_write_port(nubPort, B_DEBUG_MESSAGE_HANDED_OVER,
 			NULL, 0);
@@ -2697,6 +2773,7 @@ install_team_debugger(team_id teamID, port_id debuggerPort,
 	if (done || error != B_OK) {
 		TRACE(("install_team_debugger() done1: %ld\n",
 			(error == B_OK ? result : error)));
+		finish_debugger_change(team);
 		return (error == B_OK ? result : error);
 	}
 
@@ -2736,33 +2813,26 @@ install_team_debugger(team_id teamID, port_id debuggerPort,
 	// now adjust the debug info accordingly
 	if (error == B_OK) {
 		state = disable_interrupts();
-		GRAB_TEAM_LOCK();
+		GRAB_TEAM_DEBUG_INFO_LOCK(team->debug_info);
 
-		// look up again, to make sure the team isn't dying
+		if (team->debug_info.flags & B_TEAM_DEBUG_DEBUGGER_INSTALLED) {
-		team = team_get_team_struct_locked(teamID);
+			// there's already a debugger installed
+			error = (dontReplace ? B_OK : B_BAD_VALUE);
+			done = true;
+			result = team->debug_info.nub_port;
 
-		if (team) {
+			RELEASE_TEAM_DEBUG_INFO_LOCK(team->debug_info);
-			GRAB_TEAM_DEBUG_INFO_LOCK(team->debug_info);
+		} else {
+			install_team_debugger_init_debug_infos(team, debuggerTeam,
+				debuggerPort, nubPort, nubThread, debuggerWriteLock,
+				causingThread);
+		}
 
-			if (team->debug_info.flags & B_TEAM_DEBUG_DEBUGGER_INSTALLED) {
-				// there's already a debugger installed
-				error = (dontReplace ? B_OK : B_BAD_VALUE);
-				done = true;
-				result = team->debug_info.nub_port;
-
-				RELEASE_TEAM_DEBUG_INFO_LOCK(team->debug_info);
-			} else {
-				install_team_debugger_init_debug_infos(team, debuggerTeam,
-					debuggerPort, nubPort, nubThread, debuggerWriteLock,
-					causingThread);
-			}
-		} else
-			error = B_BAD_TEAM_ID;
-
-		RELEASE_TEAM_LOCK();
 		restore_interrupts(state);
 	}
 
+	finish_debugger_change(team);
+
 	// if everything went fine, resume the nub thread, otherwise clean up
 	if (error == B_OK && !done) {
 		resume_thread(nubThread);
@@ -2882,35 +2952,28 @@ _user_install_team_debugger(team_id teamID, port_id debuggerPort)
 status_t
 _user_remove_team_debugger(team_id teamID)
 {
+	struct team* team;
+	ConditionVariable debugChangeCondition;
+	status_t error = prepare_debugger_change(teamID, debugChangeCondition,
+		team);
+	if (error != B_OK)
+		return error;
+
+	InterruptsSpinLocker debugInfoLocker(team->debug_info.lock);
+
 	struct team_debug_info info;
+	if (team->debug_info.flags & B_TEAM_DEBUG_DEBUGGER_INSTALLED) {
+		// there's a debugger installed
+		info = team->debug_info;
+		clear_team_debug_info(&team->debug_info, false);
+	} else {
+		// no debugger installed
+		error = B_BAD_VALUE;
+	}
 
-	status_t error = B_OK;
+	debugInfoLocker.Unlock();
 
-	cpu_status state = disable_interrupts();
+	finish_debugger_change(team);
-	GRAB_TEAM_LOCK();
-
-	struct team *team = (teamID == B_CURRENT_TEAM
-		? thread_get_current_thread()->team
-		: team_get_team_struct_locked(teamID));
-
-	if (team) {
-		GRAB_TEAM_DEBUG_INFO_LOCK(team->debug_info);
-
-		if (team->debug_info.flags & B_TEAM_DEBUG_DEBUGGER_INSTALLED) {
-			// there's a debugger installed
-			info = team->debug_info;
-			clear_team_debug_info(&team->debug_info, false);
-		} else {
-			// no debugger installed
-			error = B_BAD_VALUE;
-		}
-
-		RELEASE_TEAM_DEBUG_INFO_LOCK(team->debug_info);
-	} else
-		error = B_BAD_TEAM_ID;
-
-	RELEASE_TEAM_LOCK();
-	restore_interrupts(state);
 
 	// clean up the info, if there was a debugger installed
 	if (error == B_OK)

src/system/kernel/thread.cpp

@@ -1400,6 +1400,8 @@ thread_exit(void)
 
 	struct job_control_entry *death = NULL;
 	struct death_entry* threadDeathEntry = NULL;
+	ConditionVariableEntry waitForDebuggerEntry;
+	bool waitForDebugger = false;
 
 	if (team != team_get_kernel_team()) {
 		user_debug_thread_exiting(thread);
@@ -1430,6 +1432,16 @@ thread_exit(void)
 		cachedDeathSem = team->death_sem;
 
 		if (deleteTeam) {
+			// If a debugger change is in progess for the team, we'll have to
+			// wait until it is done later.
+			GRAB_TEAM_DEBUG_INFO_LOCK(team->debug_info);
+			if (team->debug_info.debugger_changed_condition != NULL) {
+				team->debug_info.debugger_changed_condition->Add(
+					&waitForDebuggerEntry);
+				waitForDebugger = true;
+			}
+			RELEASE_TEAM_DEBUG_INFO_LOCK(team->debug_info);
+
 			struct team *parent = team->parent;
 
 			// remember who our parent was so we can send a signal
@@ -1500,6 +1512,10 @@ thread_exit(void)
 
 	// delete the team if we're its main thread
 	if (deleteTeam) {
+		// wait for a debugger change to be finished first
+		if (waitForDebugger)
+			waitForDebuggerEntry.Wait();
+
 		team_delete_team(team);
 
 		// we need to delete any death entry that made it to here