diff --git a/headers/private/media/DataExchange.h b/headers/private/media/DataExchange.h index a03c3bd5eb..e87a42abdf 100644 --- a/headers/private/media/DataExchange.h +++ b/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; diff --git a/src/kits/media/MediaEventLooper.cpp b/src/kits/media/MediaEventLooper.cpp index 08f104a92a..2f761dd06f 100644 --- a/src/kits/media/MediaEventLooper.cpp +++ b/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 */ diff --git a/src/kits/media/MediaNode.cpp b/src/kits/media/MediaNode.cpp index aeb2e14a91..2331bd3b28 100644 --- a/src/kits/media/MediaNode.cpp +++ b/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(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(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; } diff --git a/src/kits/media/MediaRoster.cpp b/src/kits/media/MediaRoster.cpp index 6a490c4ecb..ff9af8230d 100644 --- a/src/kits/media/MediaRoster.cpp +++ b/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 - // to detect dead teams. Normal communication uses ports. - static BMessage pong('PONG'); - if (message->what == 'PING') { - message->SendReply(&pong, static_cast(NULL), 2000000); - return; - } + switch (message->what) { + case 'PING': + { + // media_server plays ping-pong with the BMediaRosters + // to detect dead teams. Normal communication uses ports. + static BMessage pong('PONG'); + message->SendReply(&pong, static_cast(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(&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(); } diff --git a/src/servers/media/AppManager.cpp b/src/servers/media/AppManager.cpp index 3872b86a8c..fd3e42088d 100644 --- a/src/servers/media/AppManager.cpp +++ b/src/servers/media/AppManager.cpp @@ -222,9 +222,11 @@ void AppManager::Dump() BAutolock lock(fLocker); printf("\n"); printf("AppManager: list of applications follows:\n"); - App *app; + 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"); } diff --git a/src/servers/media/NodeManager.cpp b/src/servers/media/NodeManager.cpp index eefd2e7851..61aa148940 100644 --- a/src/servers/media/NodeManager.cpp +++ b/src/servers/media/NodeManager.cpp @@ -17,14 +17,14 @@ const char *get_node_type(node_type t); NodeManager::NodeManager() : - nextaddonid(1), - nextnodeid(1) + fNextAddOnID(1), + fNextNodeID(1), + fLocker(new BLocker("node manager locker")), + fDormantFlavorList(new List), + fAddonPathMap(new Map), + fRegisteredNodeMap(new Map), + fDefaultManager(new DefaultManager) { - fLocker = new BLocker("node manager locker"); - fDormantFlavorList = new List; - fAddonPathMap = new Map; - fRegisteredNodeMap = new Map; - 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; - nextnodeid += 1; + *nodeid = fNextNodeID; + 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; - nextaddonid += 1; + id = fNextAddOnID; + fNextAddOnID += 1; printf("NodeManager::RegisterAddon: ref-name \"%s\", assigning id %ld\n", ref.name, id); diff --git a/src/servers/media/NodeManager.h b/src/servers/media/NodeManager.h index a17d1ca865..6f2420a089 100644 --- a/src/servers/media/NodeManager.h +++ b/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_node_id nextnodeid; + media_addon_id fNextAddOnID; + media_node_id fNextNodeID; BLocker *fLocker; List *fDormantFlavorList;