shutdown_media_server: Finalize rework of synchronization

* When the user isn't requesting a custom notification, it will
be a BMediaRoster job to do it.
* Reintroduce BMediaRoster::SyncToServices, this time based on local
message passing rather than a global semaphore.
* SyncToServices is used in launch_media_server to make the process
more launch_daemon safe and faster in the average case.
* It was an error to add notifications in the media_server.
* Fixes #12717.

headers/os/media/MediaRoster.h

@@ -44,6 +44,10 @@ public:
 	// Check if the media services are running.
 	static	bool				IsRunning();
 
+	// Wait until the media_server is running or the specified timeout
+	// is reached.
+			status_t			SyncToServices(bigtime_t timeout);
+
 	// Getting common instances of system nodes:
 			status_t			GetVideoInput(media_node* _node);
 			status_t			GetAudioInput(media_node* _node);

headers/private/media/MediaRosterEx.h

@@ -95,7 +95,13 @@ public:
 		void				RegisterLocalNode(BMediaNode* node);
 		void				UnregisterLocalNode(BMediaNode* node);
 
+		void				EnableLaunchNotification(bool enable,
+								bool autoExit);
+
 private:
+		bool				fLaunchNotification;
+		bool				fAutoExit;
+
 	friend class BMediaRoster;
 };
 

headers/private/media/ServerInterface.h

@@ -40,7 +40,10 @@ enum {
 enum {
 	// local media services status notification service
 	MEDIA_ROSTER_REQUEST_NOTIFICATIONS = 2000,
-	MEDIA_ROSTER_CANCEL_NOTIFICATIONS
+	MEDIA_ROSTER_CANCEL_NOTIFICATIONS,
+
+	// used to sync with media services startup
+	MEDIA_ROSTER_REGISTER_SYNC
 };
 
 // Raw port based communication

src/kits/media/MediaDefs.cpp

@@ -29,6 +29,7 @@
 #include "DataExchange.h"
 #include "debug.h"
 #include "MediaMisc.h"
+#include "MediaRosterEx.h"
 
 
 #define META_DATA_MAX_SIZE			(16 << 20)
@@ -1301,6 +1302,15 @@ shutdown_media_server(bigtime_t timeout,
 	status_t err;
 	bool shutdown = false;
 
+	BMediaRoster* roster = BMediaRoster::Roster();
+	if (roster == NULL)
+		return B_ERROR;
+
+	if (progress == NULL && roster->Lock()) {
+		MediaRosterEx(roster)->EnableLaunchNotification(true, true);
+		roster->Unlock();
+	}
+
 	if ((err = msg.AddBool("be:_user_request", true)) != B_OK)
 		return err;
 
@@ -1323,8 +1333,8 @@ shutdown_media_server(bigtime_t timeout,
 			return rv;
 	}
 
-	shutdown = false;
 	if (be_roster->IsRunning(B_MEDIA_ADDON_SERVER_SIGNATURE)) {
+		shutdown = false;
 		BMessenger messenger(B_MEDIA_ADDON_SERVER_SIGNATURE);
 		progress_shutdown(40, progress, cookie);
 
@@ -1391,41 +1401,49 @@ launch_media_server(bigtime_t timeout,
 	if (BMediaRoster::IsRunning())
 		return B_ALREADY_RUNNING;
 
+	status_t err = B_MEDIA_SYSTEM_FAILURE;
+	BMediaRoster* roster = BMediaRoster::Roster(&err);
+	if (roster == NULL || err != B_OK)
+		return err;
+
+	if (progress == NULL && roster->Lock()) {
+		MediaRosterEx(roster)->EnableLaunchNotification(true, true);
+		roster->Unlock();
+	}
+
 	// The media_server crashed
 	if (be_roster->IsRunning(B_MEDIA_ADDON_SERVER_SIGNATURE)) {
 		progress_startup(10, progress, cookie);
 		kill_team(be_roster->TeamFor(B_MEDIA_ADDON_SERVER_SIGNATURE));
-		snooze(1000000);
 	}
 
 	// The media_addon_server crashed
 	if (be_roster->IsRunning(B_MEDIA_SERVER_SIGNATURE)) {
 		progress_startup(20, progress, cookie);
 		kill_team(be_roster->TeamFor(B_MEDIA_SERVER_SIGNATURE));
-		snooze(1000000);
 	}
 
-	status_t err = be_roster->Launch(B_MEDIA_SERVER_SIGNATURE);
-	if (err != B_OK)
-		return err;
+	progress_startup(50, progress, cookie);
 
-	err = B_MEDIA_SYSTEM_FAILURE;
-	for (int i = 0; i < 15; i++) {
-		snooze(2000000);
+	err = roster->SyncToServices(2000000);
+	if (err != B_OK) {
+		// At this point, it might be that the launch_daemon isn't
+		// restarting us, then we'll attempt at launching the server
+		// ourselves.
+		status_t err = be_roster->Launch(B_MEDIA_SERVER_SIGNATURE);
+		if (err != B_OK)
+			return err;
 
-		BMessage msg(1); // this is a hack
-		BMessage reply;
-		BMessenger messenger(B_MEDIA_ADDON_SERVER_SIGNATURE);
-
-		if (messenger.IsValid()) {
-			messenger.SendMessage(&msg, &reply, 2000000, 2000000);
-			err = B_OK;
-			break;
-		}
+		if (roster->SyncToServices(2000000) != B_OK)
+			err = B_MEDIA_SYSTEM_FAILURE;
 	}
 
 	if (err != B_OK)
 		progress_startup(90, progress, cookie);
+	else if (progress != NULL) {
+		progress_startup(100, progress, cookie);
+		err = B_OK;
+	}
 
 	return err;
 }

src/kits/media/MediaRoster.cpp

@@ -83,6 +83,11 @@ struct RosterNotification {
 };
 
 
+struct SyncedMessage {
+	BMessage* message;
+};
+
+
 struct LocalNode {
 				LocalNode(BMediaNode* local_node)
 					:
@@ -107,6 +112,7 @@ static bool sServerIsUp = false;
 static List<RosterNotification> sNotificationList;
 static BLocker sInitLocker("BMediaRoster::Roster locker");
 static List<LocalNode> sRegisteredNodes;
+static List<SyncedMessage> sSyncedMessages;
 
 
 class MediaRosterUndertaker {
@@ -148,7 +154,9 @@ using namespace BPrivate::media;
 
 BMediaRosterEx::BMediaRosterEx(status_t* _error)
 	:
-	BMediaRoster()
+	BMediaRoster(),
+	fLaunchNotification(false),
+	fAutoExit(false)
 {
 	gDormantNodeManager = new DormantNodeManager();
 	gTimeSourceObjectManager = new TimeSourceObjectManager();
@@ -230,6 +238,17 @@ BMediaRosterEx::UnregisterLocalNode(BMediaNode* node)
 }
 
 
+void
+BMediaRosterEx::EnableLaunchNotification(bool enable, bool autoExit)
+{
+	// NOTE: in theory, we should personalize it depending on each
+	// request, but we are using it just in launch/shutdown_media_server,
+	// so we are enough safe to don't care about that.
+	fLaunchNotification = enable;
+	fAutoExit = autoExit;
+}
+
+
 status_t
 BMediaRosterEx::SaveNodeConfiguration(BMediaNode* node)
 {
@@ -3344,6 +3363,26 @@ BMediaRoster::IsRunning()
 }
 
 
+status_t
+BMediaRoster::SyncToServices(bigtime_t timeout)
+{
+	BMessenger messenger(this);
+	BMessage msg(MEDIA_ROSTER_REGISTER_SYNC);
+	BMessage reply;
+
+	if (!messenger.IsValid())
+		return B_MEDIA_SYSTEM_FAILURE;
+
+	status_t ret = messenger.SendMessage(&msg, &reply, timeout, timeout);
+	if (ret == B_TIMED_OUT || reply.what == B_NO_REPLY)
+		return B_TIMED_OUT;
+	else if (ret != B_OK)
+		return ret;
+
+	return B_OK;
+}
+
+
 ssize_t
 BMediaRoster::AudioBufferSizeFor(int32 channelCount, uint32 sampleFormat,
 	float frameRate, bus_type busKind)
@@ -3432,6 +3471,20 @@ BMediaRoster::MessageReceived(BMessage* message)
 			return;
 		}
 
+		case MEDIA_ROSTER_REGISTER_SYNC:
+		{
+			BMessage reply;
+			if (sServerIsUp)
+				message->SendReply(&reply);
+			else {
+				DetachCurrentMessage();
+				SyncedMessage msg;
+				msg.message = message;
+				sSyncedMessages.Insert(msg);
+			}
+			return;
+		}
+
 		case B_SOME_APP_LAUNCHED:
 		{
 			BString mimeSig;
@@ -3493,6 +3546,22 @@ BMediaRoster::MessageReceived(BMessage* message)
 			TRACE("BMediaRoster::MessageReceived media services are"
 				" finally up.");
 
+			if (MediaRosterEx(this)->fLaunchNotification) {
+				progress_startup(100, NULL, NULL);
+				if (MediaRosterEx(this)->fAutoExit)
+					MediaRosterEx(this)->fLaunchNotification = false;
+			}
+
+			BMessage reply;
+			for (int32 i = 0; i < sSyncedMessages.CountItems(); i++) {
+				SyncedMessage* msg;
+				if (sSyncedMessages.Get(i, &msg) != true)
+					return;
+				msg->message->SendReply(&reply);
+				delete msg->message;
+				sSyncedMessages.Remove(i);
+			}
+
 			// Send the notification to our subscribers
 			for (int32 i = 0; i < sNotificationList.CountItems(); i++) {
 				RosterNotification* current;
@@ -3506,6 +3575,7 @@ BMediaRoster::MessageReceived(BMessage* message)
 					}
 				}
 			}
+			return;
 		}
 
 		case NODE_FINAL_RELEASE:

src/servers/media/media_server.cpp

@@ -143,8 +143,6 @@ ServerApp::ReadyToRun()
 {
 	gNodeManager->LoadState();
 
-	progress_startup(50, NULL, NULL);
-
 	// make sure any previous media_addon_server is gone
 	_QuitAddOnServer();
 	// and start a new one
@@ -941,11 +939,8 @@ ServerApp::MessageReceived(BMessage* msg)
 			break;
 
 		case MEDIA_SERVER_RESCAN_COMPLETED:
-		{
 			gAppManager->NotifyRosters();
-			progress_startup(100, NULL, NULL);
 			break;
-		}
 
 		case B_SOME_APP_QUIT:
 		{