From c74afb578a4715d2184a620511e49174a4642690 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Duval?= Date: Sat, 19 Oct 2013 19:23:00 +0200 Subject: [PATCH] try to avoid a media_addon_server crash on quit (bug #5863). * BMediaRosterEx::ReleaseNodeAll(): if media_server isn't available, try to contact the node locally. * BMediaRoster::UnregisterNode(): if media_server isn't available, try to get the addon_id from the local node. * BMediaNode::AddOn() as a pure virtual methode isn't available in the BMediaNode destructor. Workaround that by calling UnregisterNode() in DeleteHook() instead of the destructor. * MediaAddonServer::_DestroyInstantiatedFlavors(): give time to each flavor to clean up. --- src/kits/media/MediaNode.cpp | 9 +++++---- src/kits/media/MediaRoster.cpp | 19 ++++++++++++++---- src/servers/media_addon/MediaAddonServer.cpp | 21 ++++++++++++-------- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/kits/media/MediaNode.cpp b/src/kits/media/MediaNode.cpp index c4c67fc96a..5f7c00521d 100644 --- a/src/kits/media/MediaNode.cpp +++ b/src/kits/media/MediaNode.cpp @@ -148,12 +148,9 @@ BMediaNode::~BMediaNode() fTimeSource = NULL; } - // Attention! We do not unregister TimeSourceObject nodes, - // or delete their control ports, since they are only a + // Attention! We do not delete their control ports, since they are only a // shadow object, and the real one still exists if (0 == (fKinds & NODE_KIND_SHADOW_TIMESOURCE)) { - BMediaRoster::Roster()->UnregisterNode(this); - if (fControlPort > 0) delete_port(fControlPort); } else { @@ -829,6 +826,10 @@ BMediaNode::ApplyChangeTag(int32 previously_reserved) BMediaNode::DeleteHook(BMediaNode *node) { CALLED(); + // Attention! We do not unregister TimeSourceObject nodes, + // since they are only a shadow object, and the real one still exists + if ((fKinds & NODE_KIND_SHADOW_TIMESOURCE) == 0) + BMediaRoster::Roster()->UnregisterNode(this); 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 5c0721e40b..df17e0c1be 100644 --- a/src/kits/media/MediaRoster.cpp +++ b/src/kits/media/MediaRoster.cpp @@ -221,9 +221,18 @@ BMediaRosterEx::ReleaseNodeAll(const media_node& node) rv = QueryServer(SERVER_RELEASE_NODE_ALL, &request, sizeof(request), &reply, sizeof(reply)); if (rv != B_OK) { - ERROR("BMediaRoster::ReleaseNodeAll FAILED, node %" B_PRId32 ", port %" + ERROR("BMediaRoster::ReleaseNodeAll failed to query media_server, " + "retrying local, node %" B_PRId32 ", port %" B_PRId32 ", team %" B_PRId32 "!\n", node.node, node.port, BPrivate::current_team()); + node_final_release_command command; + rv = SendToPort(node.port, NODE_FINAL_RELEASE, &command, + sizeof(command)); + if (rv != B_OK) { + ERROR("BMediaRoster::ReleaseNodeAll FAILED, node %" B_PRId32 ", port %" + B_PRId32 ", team %" B_PRId32 "!\n", node.node, node.port, + BPrivate::current_team()); + } } return rv; } @@ -2079,13 +2088,16 @@ BMediaRoster::UnregisterNode(BMediaNode* node) BPrivate::media::notifications::NodesDeleted(&request.node_id, 1); server_unregister_node_reply reply; + reply.add_on_id = -1; status_t status = QueryServer(SERVER_UNREGISTER_NODE, &request, sizeof(request), &reply, sizeof(reply)); if (status != B_OK) { ERROR("BMediaRoster::UnregisterNode: failed to unregister node id %" B_PRId32 ", name '%s': %s\n", node->ID(), node->Name(), strerror(status)); - return status; + BMediaAddOn *addon = node->AddOn(&reply.flavor_id); + if (addon != NULL) + reply.add_on_id = addon->AddonID(); } if (reply.add_on_id != -1) { @@ -2110,7 +2122,7 @@ BMediaRoster::UnregisterNode(BMediaNode* node) // we are a friend class of BMediaNode and invalidate this member variable node->fNodeID = NODE_UNREGISTERED_ID; - return B_OK; + return status; } @@ -3309,4 +3321,3 @@ BMediaRoster::SetRunningDefault(media_node_id forDefault, BMediaRoster* BMediaRoster::sDefaultInstance = NULL; - diff --git a/src/servers/media_addon/MediaAddonServer.cpp b/src/servers/media_addon/MediaAddonServer.cpp index ddf6a36ded..7a1244f43e 100644 --- a/src/servers/media_addon/MediaAddonServer.cpp +++ b/src/servers/media_addon/MediaAddonServer.cpp @@ -315,7 +315,7 @@ MediaAddonServer::QuitRequested() BMediaRoster::CurrentRoster()->Lock(); BMediaRoster::CurrentRoster()->Quit(); - + for (iterator = fInfoMap.begin(); iterator != fInfoMap.end(); iterator++) _PutAddonIfPossible(iterator->second); @@ -579,9 +579,9 @@ MediaAddonServer::_DestroyInstantiatedFlavors(AddOnInfo& info) NodeVector::iterator iterator = info.active_flavors.begin(); for (; iterator != info.active_flavors.end(); iterator++) { media_node& node = *iterator; - + printf("node %" B_PRId32 "\n", node.node); - + if ((node.kind & B_TIME_SOURCE) != 0 && (fMediaRoster->StopTimeSource(node, 0, true) != B_OK)) { printf("MediaAddonServer::_DestroyInstantiatedFlavors couldn't stop " @@ -650,8 +650,14 @@ MediaAddonServer::_DestroyInstantiatedFlavors(AddOnInfo& info) } } } - - MediaRosterEx(fMediaRoster)->ReleaseNodeAll(node); + + if (MediaRosterEx(fMediaRoster)->ReleaseNodeAll(node) != B_OK) { + printf("MediaAddonServer::_DestroyInstantiatedFlavors " + "couldn't release node\n"); + } + + // wait a bit to let the node clean up + snooze(50000); } info.active_flavors.clear(); @@ -724,9 +730,9 @@ MediaAddonServer::_InstantiateAutostartFlavors(AddOnInfo& info) continue; else if (status != B_OK) break; - + printf("started node %" B_PRId32 "\n", index); - + status = MediaRosterEx(fMediaRoster)->RegisterNode(node, info.id, internalID); if (status != B_OK) { @@ -798,4 +804,3 @@ main() delete be_app; return 0; } -