fixing BMediaRoster::ReleaseNode()

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

headers/private/media/DataExchange.h

@@ -108,6 +108,7 @@ enum {
 	NODE_PREROLL,
 	NODE_SET_TIMESOURCE,
 	NODE_REQUEST_COMPLETED,
+	NODE_FINAL_RELEASE,
 	
 	NODE_MESSAGE_END,
 	CONSUMER_MESSAGE_START = 0x300,
@@ -878,6 +879,10 @@ struct node_set_timesource_command : public command_data
 	media_node_id timesource_id;
 };
 
+struct node_final_release_command : public command_data
+{
+};
+
 struct timesource_add_slave_node_command : public command_data
 {
 	media_node node;

src/kits/media/MediaEventLooper.cpp

@@ -19,7 +19,12 @@
 BMediaEventLooper::~BMediaEventLooper()
 {
 	CALLED();
-	// don't call Quit(); here
+
+	// don't call Quit(); here, except if the user was stupid
+	if (fControlThread != -1) {
+		printf("You MUST call BMediaEventLooper::Quit() in your destructor!\n");
+		Quit();
+	}
 }
 
 /* explicit */

src/kits/media/MediaNode.cpp

@@ -98,7 +98,6 @@ live_node_info::~live_node_info()
 BMediaNode::~BMediaNode()
 {
 	CALLED();
-	
 	// BeBook: UnregisterNode() unregisters a node from the Media Server. It's called automatically 
 	// BeBook: by the BMediaNode destructor, but it might be convenient to call it sometime before 
 	// BeBook: you delete your node instance, depending on your implementation and circumstances.
@@ -503,6 +502,25 @@ BMediaNode::HandleMessage(int32 message,
 {
 	INFO("BMediaNode::HandleMessage %#lx, node %ld\n", message, fNodeID);
 	switch (message) {
+		case NODE_FINAL_RELEASE:
+		{
+			const node_final_release_command *command = static_cast<const node_final_release_command *>(data);
+			// This is called by the media server to delete the object
+			// after is has been released by all nodes that are using it.
+			// We forward the function to the BMediaRoster, since the
+			// deletion must be done from a different thread, or the
+			// outermost destructor that will exit the thread that is
+			// reading messages from the port (this thread contex) will
+			// quit, and ~BMediaNode destructor won't be called ever.
+
+			TRACE("BMediaNode::HandleMessage NODE_FINAL_RELEASE, this %p\n", this);
+			BMessage msg(NODE_FINAL_RELEASE);
+			msg.AddPointer("node", this);
+			BMediaRoster::Roster()->PostMessage(&msg);
+			
+			return B_OK;
+		}
+	
 		case NODE_START:
 		{
 			const node_start_command *command = static_cast<const node_start_command *>(data);
@@ -736,7 +754,7 @@ BMediaNode::ApplyChangeTag(int32 previously_reserved)
 BMediaNode::DeleteHook(BMediaNode *node)
 {
 	CALLED();
-	delete this; // delete "this" or "node" ???
+	delete this; // delete "this" or "node", both are the same
 	return B_OK;
 }
 

src/kits/media/MediaRoster.cpp

@@ -417,11 +417,18 @@ BMediaRoster::ReleaseNode(const media_node & node)
 
 	server_release_node_request request;
 	server_release_node_reply reply;
+	status_t rv;
 	
 	request.node = node;
 	request.team = team;
 	
-	return QueryServer(SERVER_RELEASE_NODE, &request, sizeof(request), &reply, sizeof(reply));
+	printf("BMediaRoster::ReleaseNode, node %ld, port %ld, team %ld\n", node.node, node.port, team);
+	
+	rv = QueryServer(SERVER_RELEASE_NODE, &request, sizeof(request), &reply, sizeof(reply));
+	if (rv != B_OK) {
+		FATAL("BMediaRoster::ReleaseNode FAILED, node %ld, port %ld, team %ld!\n", node.node, node.port, team);
+	}
+	return rv;
 }
 
 BTimeSource *
@@ -1312,6 +1319,7 @@ BMediaRoster::StopWatching(const BMessenger & where,
 status_t 
 BMediaRoster::RegisterNode(BMediaNode * node)
 {
+	printf("BMediaRoster::RegisterNode %p\n", node);
 	CALLED();
 	if (node == NULL)
 		return B_BAD_VALUE;
@@ -1392,6 +1400,7 @@ BMediaRoster::RegisterNode(BMediaNode * node)
 status_t 
 BMediaRoster::UnregisterNode(BMediaNode * node)
 {
+	printf("BMediaRoster::UnregisterNode %p\n", node);
 	CALLED();
 	if (node == NULL)
 		return B_BAD_VALUE;
@@ -1422,7 +1431,11 @@ BMediaRoster::UnregisterNode(BMediaNode * node)
 	}
 	
 	if (reply.addon_id != -1) {
-		_DormantNodeManager->PutAddon(reply.addon_id);
+		// XXX This is a real big problem!
+		// XXX UnregisterNode is called by a dormant node itself, but UnregisterNode will
+		// XXX unload the dormant node image from memory when calling PutAddon
+//		_DormantNodeManager->PutAddon(reply.addon_id);
+
 		// XXX do "possible_count" increment in the server.
 	}
 
@@ -1660,6 +1673,9 @@ BMediaRosterEx::InstantiateDormantNode(media_addon_id addonid, int32 flavorid, m
 	// should be handled by RegisterNode() and UnregisterNode() now
 
 	*out_node = node->Node();
+
+	printf("BMediaRosterEx::InstantiateDormantNode: addon-id %ld, flavor_id %ld instanciated as node %ld, port %ld in team %ld\n", addonid, flavorid, out_node->node, out_node->port, team);
+
 	return B_OK;
 }
 
@@ -1716,13 +1732,15 @@ BMediaRoster::InstantiateDormantNode(const dormant_node_info & in_info,
 		FATAL("BMediaRoster::InstantiateDormantNode Error: requested B_FLAVOR_IS_GLOBAL, but dormant node has B_FLAVOR_IS_LOCAL\n");
 		return B_BAD_VALUE;
 	}
-
+#if 0
 	// If either the node, or the caller requested to make the instance global
 	// we will do it by forwarding this request into the media_addon_server, which
 	// in turn will call BMediaRosterEx::InstantiateDormantNode to create the node
 	// there and make it globally available.
 	if ((node_info.flavor_flags & B_FLAVOR_IS_GLOBAL) || (flags & B_FLAVOR_IS_GLOBAL)) {
 
+		printf("BMediaRoster::InstantiateDormantNode: creating global object in media_addon_server\n");
+
 		addonserver_instantiate_dormant_node_request request;
 		addonserver_instantiate_dormant_node_reply reply;
 		status_t rv;
@@ -1740,6 +1758,8 @@ BMediaRoster::InstantiateDormantNode(const dormant_node_info & in_info,
 	
 		return MediaRosterEx(this)->InstantiateDormantNode(in_info.addon, in_info.flavor_id, out_node);
 	}
+#endif
+	return MediaRosterEx(this)->InstantiateDormantNode(in_info.addon, in_info.flavor_id, out_node);
 }
 
 									 
@@ -2087,14 +2107,34 @@ BMediaRoster::MediaFlags(media_flags cap,
 /* virtual */ void 
 BMediaRoster::MessageReceived(BMessage * message)
 {
-	// media_server plays ping-pong with the BMediaRosters
+	switch (message->what) {
-	// to detect dead teams. Normal communication uses ports.
+		case 'PING':
-	static BMessage pong('PONG');
+		{
-	if (message->what == 'PING') {
+			// media_server plays ping-pong with the BMediaRosters
-		message->SendReply(&pong, static_cast<BHandler *>(NULL), 2000000);
+			// to detect dead teams. Normal communication uses ports.
-		return;
+			static BMessage pong('PONG');
-	}
+			message->SendReply(&pong, static_cast<BHandler *>(NULL), 2000000);
+			return;
+		}
 
+		case NODE_FINAL_RELEASE:
+		{
+			// this function is called by a BMediaNode to delete
+			// itself, as this needs to be done from another thread
+			// context, it is done here.
+			// XXX If a node is released using BMediaRoster::ReleaseNode()
+			// XXX instead of using BMediaNode::Release() / BMediaNode::Acquire()
+			// XXX fRefCount of the BMediaNode will not be correct.
+
+			BMediaNode *node;
+			message->FindPointer("node", reinterpret_cast<void **>(&node));
+
+			TRACE("BMediaRoster::MessageReceived NODE_FINAL_RELEASE releasing node %p\n", node);
+
+			node->DeleteHook(node); // we don't call Release(), see above!
+			return;
+		}
+	}
 	printf("BMediaRoster::MessageReceived: unknown message!\n");
 	message->PrintToStream();
 }

src/servers/media/AppManager.cpp

@@ -223,8 +223,10 @@ void AppManager::Dump()
 	printf("\n");
 	printf("AppManager: list of applications follows:\n");
 	App *app;
+	app_info info;
 	for (fAppMap->Rewind(); fAppMap->GetNext(&app); ) {
-		printf(" team %ld, messenger %svalid\n", app->team, app->messenger.IsValid() ? "" : "NOT ");
+		be_roster->GetRunningAppInfo(app->team, &info);
+		printf(" team %ld \"%s\", messenger %svalid\n", app->team, info.ref.name, app->messenger.IsValid() ? "" : "NOT ");
 	}
 	printf("AppManager: list end\n");
 }

src/servers/media/NodeManager.cpp

@@ -17,14 +17,14 @@
 const char *get_node_type(node_type t);
 
 NodeManager::NodeManager() :
-	nextaddonid(1),
+	fNextAddOnID(1),
-	nextnodeid(1)
+	fNextNodeID(1),
+	fLocker(new BLocker("node manager locker")),
+	fDormantFlavorList(new List<dormant_flavor_info>),
+	fAddonPathMap(new Map<media_addon_id, entry_ref>),
+	fRegisteredNodeMap(new Map<media_node_id, registered_node>),
+	fDefaultManager(new DefaultManager)
 {
-	fLocker = new BLocker("node manager locker");
-	fDormantFlavorList = new List<dormant_flavor_info>;
-	fAddonPathMap = new Map<media_addon_id, entry_ref>;
-	fRegisteredNodeMap = new Map<media_node_id, registered_node>;
-	fDefaultManager = new DefaultManager;
 }
 
 
@@ -44,7 +44,7 @@ NodeManager::RegisterNode(media_node_id *nodeid, media_addon_id addon_id, int32
 	BAutolock lock(fLocker);
 	bool b;
 	registered_node rn;
-	rn.nodeid = nextnodeid;
+	rn.nodeid = fNextNodeID;
 	rn.addon_id = addon_id;
 	rn.addon_flavor_id = addon_flavor_id;
 	strcpy(rn.name, name);
@@ -54,10 +54,10 @@ NodeManager::RegisterNode(media_node_id *nodeid, media_addon_id addon_id, int32
 	rn.globalrefcount = 1;
 	rn.teamrefcount.Insert(team, 1);
 	
-	b = fRegisteredNodeMap->Insert(nextnodeid, rn);
+	b = fRegisteredNodeMap->Insert(fNextNodeID, rn);
 	ASSERT(b);
-	*nodeid = nextnodeid;
+	*nodeid = fNextNodeID;
-	nextnodeid += 1;
+	fNextNodeID += 1;
 	TRACE("NodeManager::RegisterNode: node %ld, addon_id %ld, flavor_id %ld, name \"%s\", kinds %#Lx, port %ld, team %ld\n", *nodeid, addon_id, addon_flavor_id, name, kinds, port, team);
 	return B_OK;
 }
@@ -145,11 +145,41 @@ NodeManager::DecrementGlobalRefCount(media_node_id nodeid, team_id team)
 		ASSERT(b);
 	}
 	rn->globalrefcount -= 1;
+	
+	if (rn->globalrefcount == 0) {
+		printf("NodeManager::DecrementGlobalRefCount: detected released node is now unused, node %ld\n", nodeid);
+		FinalReleaseNode(nodeid);
+	}
+	
 	TRACE("NodeManager::DecrementGlobalRefCount leave: node %ld, team %ld, count %ld, globalcount %ld\n", nodeid, team, debug_count, rn->globalrefcount);
 	return B_OK;
 }
 
 
+void
+NodeManager::FinalReleaseNode(media_node_id nodeid)
+{
+	BAutolock lock(fLocker);
+	registered_node *rn;
+	bool b;
+	status_t rv;
+
+	TRACE("NodeManager::FinalReleaseNode enter: node %ld\n", nodeid);
+	b = fRegisteredNodeMap->Get(nodeid, &rn);
+	if (!b) {
+		FATAL("NodeManager::FinalReleaseNode: Error: node %ld not found\n", nodeid);
+		return;
+	}
+
+	node_final_release_command cmd;
+	rv = SendToPort(rn->port, NODE_FINAL_RELEASE, &cmd, sizeof(cmd));
+	if (rv != B_OK) {
+		FATAL("NodeManager::FinalReleaseNode: Error: can't send command to node %ld\n", nodeid);
+		return;
+	}
+}
+
+
 status_t
 NodeManager::GetCloneForId(media_node *node, media_node_id nodeid, team_id team)
 {
@@ -445,8 +475,8 @@ NodeManager::RegisterAddon(const entry_ref &ref, media_addon_id *newid)
 {
 	BAutolock lock(fLocker);
 	media_addon_id id;
-	id = nextaddonid;
+	id = fNextAddOnID;
-	nextaddonid += 1;
+	fNextAddOnID += 1;
 	
 	printf("NodeManager::RegisterAddon: ref-name \"%s\", assigning id %ld\n", ref.name, id);
 	

src/servers/media/NodeManager.h

@@ -68,6 +68,7 @@ public:
 	status_t GetDormantNodeInfo(dormant_node_info *node_info, const media_node &node);
 	status_t IncrementGlobalRefCount(media_node_id nodeid, team_id team);
 	status_t DecrementGlobalRefCount(media_node_id nodeid, team_id team);
+	void FinalReleaseNode(media_node_id nodeid);
 
 	/* Add media_node_id of all live nodes to the message
 	 * int32 "media_node_id" (multiple items)
@@ -95,8 +96,8 @@ public:
 	void CleanupTeam(team_id team);
 	
 private:
-	media_addon_id nextaddonid;
+	media_addon_id fNextAddOnID;
-	media_node_id nextnodeid;
+	media_node_id fNextNodeID;
 	
 	BLocker *fLocker;
 	List<dormant_flavor_info> *fDormantFlavorList;