From 7bcdb3624951ebf640098d2a860cef7d9557e1df Mon Sep 17 00:00:00 2001 From: Dario Casalinuovo Date: Mon, 25 May 2015 12:43:27 +0200 Subject: [PATCH] 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 | 10 +++ src/kits/media/MediaRoster.cpp | 81 ++++++++++++++----------- src/kits/media/TimeSource.cpp | 6 -- src/servers/media/NodeManager.cpp | 50 ++++++++++++++- src/servers/media/NodeManager.h | 6 ++ src/servers/media/media_server.cpp | 13 +++- 6 files changed, 122 insertions(+), 44 deletions(-) diff --git a/headers/private/media/ServerInterface.h b/headers/private/media/ServerInterface.h index 13249ebae8..5b04b601bd 100644 --- a/headers/private/media/ServerInterface.h +++ b/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 diff --git a/src/kits/media/MediaRoster.cpp b/src/kits/media/MediaRoster.cpp index 8201be0b75..18a078647b 100644 --- a/src/kits/media/MediaRoster.cpp +++ b/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 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 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; + if (result == B_OK) { + // Notify the server + server_set_node_timesource_request request; + server_set_node_timesource_reply reply; + + 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); + } } - - // 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; - } - - TRACE("BMediaRoster::SetTimeSourceFor: node %" B_PRId32 " time source %" - B_PRId32 " leave\n", node, time_source); - return result; } diff --git a/src/kits/media/TimeSource.cpp b/src/kits/media/TimeSource.cpp index f3f6ad224f..89f1e57913 100644 --- a/src/kits/media/TimeSource.cpp +++ b/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); diff --git a/src/servers/media/NodeManager.cpp b/src/servers/media/NodeManager.cpp index f29fe3e347..d9315a2db9 100644 --- a/src/servers/media/NodeManager.cpp +++ b/src/servers/media/NodeManager.cpp @@ -1,4 +1,5 @@ /* + * Copyright (c) 2015 Dario Casalinuovo * Copyright (c) 2002, 2003 Marcus Overhagen * * Permission is hereby granted, free of charge, to any person obtaining @@ -30,6 +31,7 @@ #include "NodeManager.h" +#include #include #include #include @@ -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, + ¤tNode) == 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); +} diff --git a/src/servers/media/NodeManager.h b/src/servers/media/NodeManager.h index 83569ec851..fab52e4f08 100644 --- a/src/servers/media/NodeManager.h +++ b/src/servers/media/NodeManager.h @@ -27,6 +27,7 @@ typedef std::vector 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 @@ public: 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 @@ public: 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 @@ public: private: status_t _AcquireNodeReference(media_node_id id, team_id team); + void _NotifyTimeSource(registered_node& node); private: typedef std::map NodeMap; diff --git a/src/servers/media/media_server.cpp b/src/servers/media/media_server.cpp index def54675db..b7fdfc7c8a 100644 --- a/src/servers/media/media_server.cpp +++ b/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(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<