* Changed team_get_process_group_locked() to have a session parameter instead

of a team to avoid confusion. It now also accepts a NULL session pointer in
  which case the actual group's session doesn't matter.
* Fixed the race condition in send_signal_etc() that could allow accessing an
  invalid team pointer.
* Jerome's earlier change already fixed bug #841, and this also fixed bug #855.


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

headers/private/kernel/team.h

@@ -20,7 +20,7 @@ team_id team_create_team(const char *path, const char *name, char **args, int ar
 status_t wait_for_team(team_id id, status_t *returnCode);
 void team_remove_team(struct team *team, struct process_group **_freeGroup);
 void team_delete_team(struct team *team);
-struct process_group *team_get_process_group_locked(struct team *team, pid_t id);
+struct process_group *team_get_process_group_locked(struct process_session *session, pid_t id);
 void team_delete_process_group(struct process_group *group);
 struct team *team_get_kernel_team(void);
 team_id team_get_kernel_team_id(void);

src/system/kernel/signal.c

@@ -304,52 +304,42 @@ send_signal_etc(pid_t id, uint signal, uint32 flags)
 		// send a signal to the specified process group
 		// (the absolute value of the id)
 
-		struct team *team = NULL;
+		struct process_group *group;
 		
 		// TODO: handle -1 correctly
 		if (id == 0 || id == -1) {
 			// send a signal to the current team
-			team = thread_get_current_thread()->team;
+			id = thread_get_current_thread()->team->main_thread->id;
-			id = team->main_thread->id;
+		} else
-		} else {
 			id = -id;
-			GRAB_THREAD_LOCK();
-			thread = thread_get_thread_struct_locked(id);
-			if (thread)
-				team = thread->team;
-			RELEASE_THREAD_LOCK();
-		}
 
-		if (team != NULL) {
+		GRAB_TEAM_LOCK();
-			struct process_group *group;
+
-			GRAB_TEAM_LOCK();
+		group = team_get_process_group_locked(NULL, id);
-	
+		if (group != NULL) {
-			group = team_get_process_group_locked(team, id);
+			struct team *team, *next;
-			
+
-			if (group != NULL) {
+			// Send a signal to all teams in this process group
-				struct team *team, *next;
+
-	
+			for (team = group->teams; team != NULL; team = next) {
-				// we need a safe way to get from the thread to the process group
+				next = team->group_next;
-				
+				id = team->main_thread->id;
-				for (team = group->teams; team != NULL; team = next) {
+
-					next = team->group_next;
+				GRAB_THREAD_LOCK();
-					id = team->main_thread->id;
+
-					
+				thread = thread_get_thread_struct_locked(id);
-					GRAB_THREAD_LOCK();
+				if (thread != NULL) {
-	
+					// we don't stop because of an error sending the signal; we
-					thread = thread_get_thread_struct_locked(id);
+					// rather want to send as much signals as possible
-					if (thread != NULL) {
+					status = deliver_signal(thread, signal, flags);
-						// we don't stop because of an error sending the signal; we
-						// rather want to send as much signals as possible
-						status = deliver_signal(thread, signal, flags);
-					}
-	
-					RELEASE_THREAD_LOCK();
 				}
-				
+
+				RELEASE_THREAD_LOCK();
 			}
+
 			RELEASE_TEAM_LOCK();
 		}
+
 		GRAB_THREAD_LOCK();
 	}		
 

src/system/kernel/team.c

@@ -1235,7 +1235,7 @@ update_wait_for_any(struct team *team, thread_id child, int32 change)
 
 	if (child < 0) {
 		// we wait for all children of the specified process group
-		group = team_get_process_group_locked(team, -child);
+		group = team_get_process_group_locked(team->group->session, -child);
 	} else {
 		// we wait for any children of the current team's group
 		group = team->group;
@@ -1320,7 +1320,7 @@ get_death_entry(struct team *team, pid_t child, struct death_entry *death,
 		return get_team_death_entry(team, child, death, _freeDeath);
 	} else if (child < 0) {
 		// we wait for all children of the specified process group
-		group = team_get_process_group_locked(team, -child);
+		group = team_get_process_group_locked(team->group->session, -child);
 		if (group == NULL)
 			return B_BAD_THREAD_ID;
 	} else {
@@ -1558,14 +1558,14 @@ team_get_team_struct_locked(team_id id)
  */
 
 struct process_group *
-team_get_process_group_locked(struct team *team, pid_t id)
+team_get_process_group_locked(struct process_session *session, pid_t id)
 {
 	struct process_group *group;
 	struct team_key key;
 	key.id = id;
 
 	group = (struct process_group *)hash_lookup(sGroupHash, &key);
-	if (group != NULL && team->group->session == group->session)
+	if (group != NULL && (session == NULL || session == group->session))
 		return group;
 
 	return NULL;
@@ -2360,7 +2360,8 @@ _user_setpgid(pid_t processID, pid_t groupID)
 			// check if this team can have the group ID; there must be one matching
 			// process ID in the team's session
 
-			struct process_group *group = team_get_process_group_locked(team, groupID);
+			struct process_group *group =
+				team_get_process_group_locked(team->group->session, groupID);
 			if (group) {
 				// we got a group, let's move the team there
 				remove_team_from_group(team, &freeGroup);