media_server: Improve BTimeSource slave nodes management

The media_server is now able to remember the timesource associated to
a certain registered_node and always remove it when the owner
application crash, Fixes Ticket #11852

headers/private/media/ServerInterface.h

@@ -79,6 +79,7 @@ enum {
 	SERVER_REGISTER_DORMANT_NODE,
 	SERVER_GET_DORMANT_NODES,
 	SERVER_GET_DORMANT_FLAVOR_INFO,
+	SERVER_SET_NODE_TIMESOURCE,
 	SERVER_MESSAGE_END,
 
 	NODE_MESSAGE_START = 0x200,
@@ -390,6 +391,7 @@ struct server_change_flavor_instances_count_reply : reply_data {
 struct server_register_node_request : request_data {
 	media_addon_id			add_on_id;
 	int32					flavor_id;
+	media_node_id			timesource_id;
 	char					name[B_MEDIA_NAME_LENGTH];
 	uint64					kinds;
 	port_id					port;
@@ -634,6 +636,14 @@ struct server_register_dormant_node_command : command_data {
 		// a flattened dormant_flavor_info, flattened_size large
 };
 
+struct server_set_node_timesource_request : request_data {
+	media_node_id			node_id;
+	media_node_id			timesource_id;
+};
+
+struct server_set_node_timesource_reply : reply_data {
+};
+
 
 // #pragma mark - buffer producer commands
 

src/kits/media/MediaRoster.cpp

@@ -1,6 +1,7 @@
 /*
- * Copyright 2008 Maurice Kalinowski, haiku@kaldience.com
+ * Copyright 2015 Dario Casalinuovo
  * Copyright 2009-2012, Axel Dörfler, axeld@pinc-software.de.
+ * Copyright 2008 Maurice Kalinowski, haiku@kaldience.com
  *
  * All rights reserved. Distributed under the terms of the MIT License.
  */
@@ -1972,6 +1973,7 @@ BMediaRosterEx::RegisterNode(BMediaNode* node, media_addon_id addOnID,
 	request.kinds = node->Kinds();
 	request.port = node->ControlPort();
 	request.team = BPrivate::current_team();
+	request.timesource_id = node->fTimeSourceID;
 
 	TRACE("BMediaRoster::RegisterNode: sending SERVER_REGISTER_NODE: port "
 		"%" B_PRId32 ", kinds 0x%" B_PRIx64 ", team %" B_PRId32 ", name '%s'\n",
@@ -2176,44 +2178,51 @@ BMediaRoster::SetTimeSourceFor(media_node_id node, media_node_id time_source)
 		return B_BAD_VALUE;
 
 	media_node clone;
-	status_t rv, result;
-
-	TRACE("BMediaRoster::SetTimeSourceFor: node %" B_PRId32 " will be assigned "
-		"time source %" B_PRId32 "\n", node, time_source);
-	TRACE("BMediaRoster::SetTimeSourceFor: node %" B_PRId32 " time source %"
-		B_PRId32 " enter\n", node, time_source);
-
-	// we need to get a clone of the node to have a port id
-	rv = GetNodeFor(node, &clone);
-	if (rv != B_OK) {
-		ERROR("BMediaRoster::SetTimeSourceFor, GetNodeFor failed, node id %"
-			B_PRId32 "\n", node);
-		return B_ERROR;
-	}
-
-	// we just send the request to set time_source-id as timesource to the node,
-	// the NODE_SET_TIMESOURCE handler code will do the real assignment
-	result = B_OK;
-	node_set_timesource_command cmd;
-	cmd.timesource_id = time_source;
-	rv = SendToPort(clone.port, NODE_SET_TIMESOURCE, &cmd, sizeof(cmd));
-	if (rv != B_OK) {
-		ERROR("BMediaRoster::SetTimeSourceFor, sending NODE_SET_TIMESOURCE "
-			"failed, node id %" B_PRId32 "\n", node);
-		result = B_ERROR;
+	// We need to get a clone of the node to have a port id
+	status_t result = GetNodeFor(node, &clone);
+	if (result == B_OK) {
+		// We just send the request to set time_source-id as
+		// timesource to the node, the NODE_SET_TIMESOURCE handler
+		// code will do the real assignment.
+		result = B_OK;
+		node_set_timesource_command cmd;
+		cmd.timesource_id = time_source;
+		result = SendToPort(clone.port, NODE_SET_TIMESOURCE,
+			&cmd, sizeof(cmd));
+		if (result != B_OK) {
+			ERROR("BMediaRoster::SetTimeSourceFor"
+				"sending NODE_SET_TIMESOURCE failed, node id %"
+				B_PRId32 "\n", clone.node);
+		}
+		// We release the clone
+		result = ReleaseNode(clone);
+		if (result != B_OK) {
+			ERROR("BMediaRoster::SetTimeSourceFor, ReleaseNode failed,"
+				" node id %" B_PRId32 "\n", clone.node);
+		}
+	} else {
+		ERROR("BMediaRoster::SetTimeSourceFor GetCloneForID failed, "
+			"node id %" B_PRId32 "\n", node);
 	}
 
-	// we release the clone
-	rv = ReleaseNode(clone);
-	if (rv != B_OK) {
-		ERROR("BMediaRoster::SetTimeSourceFor, ReleaseNode failed, node id %"
-			B_PRId32 "\n", node);
-		result = B_ERROR;
-	}
+	if (result == B_OK) {
+		// Notify the server
+		server_set_node_timesource_request request;
+		server_set_node_timesource_reply reply;
 
-	TRACE("BMediaRoster::SetTimeSourceFor: node %" B_PRId32 " time source %"
-		B_PRId32 " leave\n", node, time_source);
+		request.node_id = node;
+		request.timesource_id = time_source;
 
+		result = QueryServer(SERVER_SET_NODE_TIMESOURCE, &request,
+			sizeof(request), &reply, sizeof(reply));
+		if (result != B_OK) {
+			ERROR("BMediaRoster::SetTimeSourceFor, sending NODE_SET_TIMESOURCE "
+				"failed, node id %" B_PRId32 "\n", node);
+		} else {
+			TRACE("BMediaRoster::SetTimeSourceFor: node %" B_PRId32 " time source %"
+				B_PRId32 " OK\n", node, time_source);
+		}
+	}
 	return result;
 }
 

src/kits/media/TimeSource.cpp

@@ -545,9 +545,6 @@ BTimeSource::AddMe(BMediaNode* node)
 void
 BTimeSource::DirectAddMe(const media_node& node)
 {
-	// XXX this code has race conditions and is pretty dumb, and it
-	// XXX won't detect nodes that crash and don't remove themself.
-
 	CALLED();
 	ASSERT(fSlaveNodes != NULL);
 	BAutolock lock(fSlaveNodes);
@@ -582,9 +579,6 @@ BTimeSource::DirectAddMe(const media_node& node)
 void
 BTimeSource::DirectRemoveMe(const media_node& node)
 {
-	// XXX this code has race conditions and is pretty dumb, and it
-	// XXX won't detect nodes that crash and don't remove themself.
-
 	CALLED();
 	ASSERT(fSlaveNodes != NULL);
 	BAutolock lock(fSlaveNodes);

src/servers/media/NodeManager.cpp

@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2015 Dario Casalinuovo
  * Copyright (c) 2002, 2003 Marcus Overhagen <Marcus@Overhagen.de>
  *
  * Permission is hereby granted, free of charge, to any person obtaining
@@ -30,6 +31,7 @@
 
 #include "NodeManager.h"
 
+#include <Application.h>
 #include <Autolock.h>
 #include <Entry.h>
 #include <MediaAddOn.h>
@@ -144,12 +146,13 @@ NodeManager::RescanDefaultNodes()
 status_t
 NodeManager::RegisterNode(media_addon_id addOnID, int32 flavorID,
 	const char* name, uint64 kinds, port_id port, team_id team,
-	media_node_id* _nodeID)
+	media_node_id timesource, media_node_id* _nodeID)
 {
 	BAutolock _(this);
 
 	registered_node node;
 	node.node_id = fNextNodeID;
+	node.timesource_id = timesource;
 	node.add_on_id = addOnID;
 	node.flavor_id = flavorID;
 	strlcpy(node.name, name, sizeof(node.name));
@@ -1092,6 +1095,24 @@ NodeManager::GetDormantFlavorInfoFor(media_addon_id addOnID, int32 flavorID,
 // #pragma mark - Misc.
 
 
+status_t
+NodeManager::SetNodeTimeSource(media_node_id node,
+	media_node_id timesource)
+{
+	BAutolock _(this);
+
+	NodeMap::iterator found = fNodeMap.find(node);
+	if (found == fNodeMap.end()) {
+		ERROR("NodeManager::SetNodeTimeSource: node %"
+			B_PRId32 " not found\n", node);
+		return B_ERROR;
+	}
+	registered_node& registeredNode = found->second;
+	registeredNode.timesource_id = timesource;
+	return B_OK;
+}
+
+
 void
 NodeManager::CleanupTeam(team_id team)
 {
@@ -1120,6 +1141,8 @@ NodeManager::CleanupTeam(team_id team)
 		if (node.containing_team == team) {
 			PRINT(1, "NodeManager::CleanupTeam: removing node id %" B_PRId32
 				", team %" B_PRId32 "\n", node.node_id, team);
+			// Ensure the slave node is removed from it's timesource
+			_NotifyTimeSource(node);
 			fNodeMap.erase(remove);
 			BPrivate::media::notifications::NodesDeleted(&node.node_id, 1);
 			continue;
@@ -1137,6 +1160,8 @@ NodeManager::CleanupTeam(team_id team)
 				PRINT(1, "NodeManager::CleanupTeam: removing node id %"
 					B_PRId32 " that has no teams\n", node.node_id);
 
+				// Ensure the slave node is removed from it's timesource
+				_NotifyTimeSource(node);
 				fNodeMap.erase(remove);
 				BPrivate::media::notifications::NodesDeleted(&node.node_id, 1);
 			} else
@@ -1340,3 +1365,26 @@ NodeManager::_AcquireNodeReference(media_node_id id, team_id team)
 		node.ref_count, node.team_ref_count.find(team)->second);
 	return B_OK;
 }
+
+
+void
+NodeManager::_NotifyTimeSource(registered_node& node)
+{
+	team_id team = be_app->Team();
+	media_node timeSource;
+	// Ensure the timesource ensure still exists
+	if (GetCloneForID(node.timesource_id, team, &timeSource) != B_OK)
+		return;
+
+	media_node currentNode;
+	if (GetCloneForID(node.node_id, team,
+		&currentNode) == B_OK) {
+		timesource_remove_slave_node_command cmd;
+		cmd.node = currentNode;
+		// Notify slave node removal to owner timesource
+		SendToPort(timeSource.port, TIMESOURCE_REMOVE_SLAVE_NODE,
+			&cmd, sizeof(cmd));
+		ReleaseNode(timeSource, team);
+	}
+	ReleaseNode(currentNode, team);
+}

src/servers/media/NodeManager.h

@@ -27,6 +27,7 @@ typedef std::vector<live_node_info> LiveNodeList;
 
 struct registered_node {
 	media_node_id			node_id;
+	media_node_id			timesource_id;
 	media_addon_id			add_on_id;
 	int32					flavor_id;
 	char					name[B_MEDIA_NAME_LENGTH];
@@ -73,6 +74,7 @@ class NodeManager : BLocker {
 			status_t			RegisterNode(media_addon_id addOnID,
 									int32 flavorID, const char* name,
 									uint64 kinds, port_id port, team_id team,
+									media_node_id timesource,
 									media_node_id* _nodeID);
 			status_t			UnregisterNode(media_node_id nodeID,
 									team_id team, media_addon_id* addOnID,
@@ -141,6 +143,9 @@ class NodeManager : BLocker {
 									 int32 flavorID,
 									 dormant_flavor_info* flavorInfo);
 
+			status_t			SetNodeTimeSource(media_node_id node,
+									media_node_id timesource);
+
 			void				CleanupTeam(team_id team);
 
 			status_t			LoadState();
@@ -151,6 +156,7 @@ class NodeManager : BLocker {
 private:
 			status_t			_AcquireNodeReference(media_node_id id,
 									team_id team);
+			void				_NotifyTimeSource(registered_node& node);
 
 private:
 			typedef std::map<media_addon_id, registered_node> NodeMap;

src/servers/media/media_server.cpp

@@ -447,7 +447,7 @@ ServerApp::_HandleMessage(int32 code, const void* data, size_t size)
 
 			status_t status = gNodeManager->RegisterNode(request.add_on_id,
 				request.flavor_id, request.name, request.kinds, request.port,
-				request.team, &reply.node_id);
+				request.team, request.timesource_id, &reply.node_id);
 			request.SendReply(status, &reply, sizeof(reply));
 			break;
 		}
@@ -580,6 +580,17 @@ ServerApp::_HandleMessage(int32 code, const void* data, size_t size)
 			break;
 		}
 
+		case SERVER_SET_NODE_TIMESOURCE:
+		{
+			const server_set_node_timesource_request& request
+				= *static_cast<const server_set_node_timesource_request*>(data);
+			server_set_node_timesource_reply reply;
+			status_t result = gNodeManager->SetNodeTimeSource(request.node_id,
+				request.timesource_id);
+			request.SendReply(result, &reply, sizeof(reply));
+			break;
+		}
+
 		case SERVER_REGISTER_ADD_ON:
 		{
 			const server_register_add_on_request& request = *static_cast<