[haiku-commits] haiku: hrev49583 - in src: servers/launch kits/app

  • From: mmlr@xxxxxxxx
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Wed, 26 Aug 2015 22:25:28 +0200 (CEST)

hrev49583 adds 2 changesets to branch 'master'
old head: ae9cbf9c4e167470b47964059e90c2b0881367eb
new head: 14156a33ac0233f4233546080d6b975618b331fa
overview:
http://cgit.haiku-os.org/haiku/log/?qt=range&q=14156a33ac02+%5Eae9cbf9c4e16

----------------------------------------------------------------------------

ee5575d9b34b: BRoster: Apply no-registrar mode in a few more cases.

Avoids some more attempts at communicating with the registrar if the
no-registrar flag has been set.

14156a33ac02: launch_daemon: Delegate launch data replies to Job.

Previously the LaunchDaemon would send out its own team id when a given
job was not yet launched, leading to invalid BMessengers once the port
owner changed to the actually launched team.

The launch of the target team and the launch data replies were also not
synchronized, which could lead to the launched team getting a reply
pointing to the launch_daemon when requesting data for itself. This is
the case for the BRoster init of the registrar. The fix in hrev49561
therefore didn't always work, because the registrar would sometimes get
the launch_daemon team id instead of the id of itself. It would later
try talking to the launch_daemon, which obviously never replied, leading
to #12237.

The LaunchDaemon now delegates the launch data reply to the Job instead.
The Job either replies directly, in case it has already been launched,
or queues the reply for when the launch completes. This causes launch
data requesters to block until the launch attempt is completed, but
won't block the LaunchDaemon message loop.

This commit introduces the seperate fLaunchStatus to properly handle the
ambiguity of fTeam being < 0, which is the case for both, when no launch
was attempted and when the launch failed. This new field now determines
what IsLaunched() returns and how launch data replies are handled.

The new launch status is additionally protected by the launch status
lock, which will later probably be made broader in scope to protect
against race conditions once service monitoring is implemented.

[ Michael Lotz <mmlr@xxxxxxxx> ]

----------------------------------------------------------------------------

4 files changed, 112 insertions(+), 27 deletions(-)
src/kits/app/Roster.cpp | 7 ++-
src/servers/launch/Job.cpp | 85 ++++++++++++++++++++++++++++++---
src/servers/launch/Job.h | 13 +++++
src/servers/launch/LaunchDaemon.cpp | 34 +++++++------

############################################################################

Commit: ee5575d9b34ba547c0677a9b90bda2ebc066acee
URL: http://cgit.haiku-os.org/haiku/commit/?id=ee5575d9b34b
Author: Michael Lotz <mmlr@xxxxxxxx>
Date: Wed Aug 26 19:33:41 2015 UTC

BRoster: Apply no-registrar mode in a few more cases.

Avoids some more attempts at communicating with the registrar if the
no-registrar flag has been set.

----------------------------------------------------------------------------

diff --git a/src/kits/app/Roster.cpp b/src/kits/app/Roster.cpp
index 1a8c769..deaef0b 100644
--- a/src/kits/app/Roster.cpp
+++ b/src/kits/app/Roster.cpp
@@ -2068,7 +2068,7 @@ BRoster::_ResolveApp(const char* inType, entry_ref* ref,
}

// create meta mime
- if (error == B_OK) {
+ if (!fNoRegistrar && error == B_OK) {
BPath path;
if (path.SetTo(&appRef) == B_OK)
create_app_meta_mime(path.Path(), false, true, false);
@@ -2077,7 +2077,7 @@ BRoster::_ResolveApp(const char* inType, entry_ref* ref,
// set the app hint on the type -- but only if the file has the
// respective signature, otherwise unset the app hint
BAppFileInfo appFileInfo;
- if (error == B_OK) {
+ if (!fNoRegistrar && error == B_OK) {
char signature[B_MIME_TYPE_LENGTH];
if (appFileInfo.SetTo(&appFile) == B_OK
&& appFileInfo.GetSignature(signature) == B_OK) {
@@ -2482,6 +2482,9 @@ BRoster::_GetFileType(const entry_ref* file, BNodeInfo*
nodeInfo,
if (nodeInfo->GetType(mimeType) == B_OK)
return B_OK;

+ if (fNoRegistrar)
+ return B_NO_INIT;
+
// Try to update the file's MIME info and just read the updated type.
// If that fails, sniff manually.
BPath path;

############################################################################

Revision: hrev49583
Commit: 14156a33ac0233f4233546080d6b975618b331fa
URL: http://cgit.haiku-os.org/haiku/commit/?id=14156a33ac02
Author: Michael Lotz <mmlr@xxxxxxxx>
Date: Wed Aug 26 19:39:15 2015 UTC

Ticket: https://dev.haiku-os.org/ticket/12237

launch_daemon: Delegate launch data replies to Job.

Previously the LaunchDaemon would send out its own team id when a given
job was not yet launched, leading to invalid BMessengers once the port
owner changed to the actually launched team.

The launch of the target team and the launch data replies were also not
synchronized, which could lead to the launched team getting a reply
pointing to the launch_daemon when requesting data for itself. This is
the case for the BRoster init of the registrar. The fix in hrev49561
therefore didn't always work, because the registrar would sometimes get
the launch_daemon team id instead of the id of itself. It would later
try talking to the launch_daemon, which obviously never replied, leading
to #12237.

The LaunchDaemon now delegates the launch data reply to the Job instead.
The Job either replies directly, in case it has already been launched,
or queues the reply for when the launch completes. This causes launch
data requesters to block until the launch attempt is completed, but
won't block the LaunchDaemon message loop.

This commit introduces the seperate fLaunchStatus to properly handle the
ambiguity of fTeam being < 0, which is the case for both, when no launch
was attempted and when the launch failed. This new field now determines
what IsLaunched() returns and how launch data replies are handled.

The new launch status is additionally protected by the launch status
lock, which will later probably be made broader in scope to protect
against race conditions once service monitoring is implemented.

----------------------------------------------------------------------------

diff --git a/src/servers/launch/Job.cpp b/src/servers/launch/Job.cpp
index 7432921..5cb9c98 100644
--- a/src/servers/launch/Job.cpp
+++ b/src/servers/launch/Job.cpp
@@ -27,8 +27,11 @@ Job::Job(const char* name)
fCreateDefaultPort(false),
fInitStatus(B_NO_INIT),
fTeam(-1),
- fTarget(NULL)
+ fLaunchStatus(B_NO_INIT),
+ fTarget(NULL),
+ fPendingLaunchDataReplies(0, false)
{
+ mutex_init(&fLaunchStatusLock, "launch status lock");
}


@@ -40,8 +43,12 @@ Job::Job(const Job& other)
fCreateDefaultPort(other.CreateDefaultPort()),
fInitStatus(B_NO_INIT),
fTeam(-1),
- fTarget(other.Target())
+ fLaunchStatus(B_NO_INIT),
+ fTarget(other.Target()),
+ fPendingLaunchDataReplies(0, false)
{
+ mutex_init(&fLaunchStatusLock, "launch status lock");
+
fCondition = other.fCondition;
// TODO: copy events
//fEvent = other.fEvent;
@@ -343,16 +350,21 @@ Job::Launch()
// Launch by signature
BString signature("application/");
signature << Name();
- return BRoster::Private().Launch(signature.String(), NULL, NULL,
- 0, NULL, &environment[0], &fTeam);
+
+ status_t status = BRoster::Private().Launch(signature.String(),
NULL,
+ NULL, 0, NULL, &environment[0], &fTeam);
+ _SetLaunchStatus(status);
+ return status;
}

// Build argument vector

entry_ref ref;
status_t status = get_ref_for_path(fArguments.StringAt(0).String(),
&ref);
- if (status != B_OK)
+ if (status != B_OK) {
+ _SetLaunchStatus(status);
return status;
+ }

std::vector<const char*> args;

@@ -365,15 +377,28 @@ Job::Launch()
}

// Launch via entry_ref
- return BRoster::Private().Launch(NULL, &ref, NULL, count, &args[0],
+ status = BRoster::Private().Launch(NULL, &ref, NULL, count, &args[0],
&environment[0], &fTeam);
+ _SetLaunchStatus(status);
+ return status;
}


bool
Job::IsLaunched() const
{
- return fTeam >= 0;
+ return fLaunchStatus != B_NO_INIT;
+}
+
+
+status_t
+Job::HandleGetLaunchData(BMessage* message)
+{
+ MutexLocker launchLocker(fLaunchStatusLock);
+ if (IsLaunched())
+ return _SendLaunchDataReply(message);
+
+ return fPendingLaunchDataReplies.AddItem(message) ? B_OK : B_NO_MEMORY;
}


@@ -434,3 +459,49 @@ Job::_AddStringList(std::vector<const char*>& array, const
BStringList& list)
array.push_back(list.StringAt(index).String());
}
}
+
+
+void
+Job::_SetLaunchStatus(status_t launchStatus)
+{
+ MutexLocker launchLocker(fLaunchStatusLock);
+ fLaunchStatus = launchStatus != B_NO_INIT ? launchStatus : B_ERROR;
+ launchLocker.Unlock();
+
+ _SendPendingLaunchDataReplies();
+}
+
+
+status_t
+Job::_SendLaunchDataReply(BMessage* message)
+{
+ BMessage reply(fTeam < 0 ? fTeam : (uint32)B_OK);
+ if (reply.what == B_OK) {
+ reply.AddInt32("team", fTeam);
+
+ PortMap::const_iterator iterator = fPortMap.begin();
+ for (; iterator != fPortMap.end(); iterator++) {
+ BString name;
+ if (iterator->second.HasString("name"))
+ name << iterator->second.GetString("name") <<
"_";
+ name << "port";
+
+ reply.AddInt32(name.String(),
+ iterator->second.GetInt32("port", -1));
+ }
+ }
+
+ message->SendReply(&reply);
+ delete message;
+ return B_OK;
+}
+
+
+void
+Job::_SendPendingLaunchDataReplies()
+{
+ for (int32 i = 0; i < fPendingLaunchDataReplies.CountItems(); i++)
+ _SendLaunchDataReply(fPendingLaunchDataReplies.ItemAt(i));
+
+ fPendingLaunchDataReplies.MakeEmpty();
+}
diff --git a/src/servers/launch/Job.h b/src/servers/launch/Job.h
index e2ee0b4..63afeae 100644
--- a/src/servers/launch/Job.h
+++ b/src/servers/launch/Job.h
@@ -15,6 +15,8 @@
#include <OS.h>
#include <StringList.h>

+#include <locks.h>
+

using namespace BSupportKit;
class BMessage;
@@ -68,6 +70,8 @@ public:
status_t Launch();
bool IsLaunched() const;

+ status_t
HandleGetLaunchData(BMessage* message);
+
protected:
virtual status_t Execute();

@@ -78,6 +82,11 @@ private:
void
_AddStringList(std::vector<const char*>& array,
const
BStringList& list);

+ void
_SetLaunchStatus(status_t launchStatus);
+
+ status_t
_SendLaunchDataReply(BMessage* message);
+ void
_SendPendingLaunchDataReplies();
+
private:
BStringList fArguments;
BStringList fRequirements;
@@ -87,8 +96,12 @@ private:
PortMap fPortMap;
status_t fInitStatus;
team_id fTeam;
+ status_t fLaunchStatus;
+ mutex fLaunchStatusLock;
::Target* fTarget;
::Condition* fCondition;
+ BObjectList<BMessage>
+
fPendingLaunchDataReplies;
};


diff --git a/src/servers/launch/LaunchDaemon.cpp
b/src/servers/launch/LaunchDaemon.cpp
index b64d40a..f9733f9 100644
--- a/src/servers/launch/LaunchDaemon.cpp
+++ b/src/servers/launch/LaunchDaemon.cpp
@@ -483,31 +483,29 @@ LaunchDaemon::_HandleGetLaunchData(BMessage* message)
launchJob = false;
}
}
- }
+ } else
+ launchJob = false;

+ bool ownsMessage = false;
if (reply.what == B_OK) {
- // If the job has not been launched yet, we'll pass on our
- // team here. The rationale behind this is that this team
- // will temporarily own the synchronous reply ports.
- reply.AddInt32("team", job->Team() < 0
- ? current_team() : job->Team());
-
- PortMap::const_iterator iterator = job->Ports().begin();
- for (; iterator != job->Ports().end(); iterator++) {
- BString name;
- if (iterator->second.HasString("name"))
- name << iterator->second.GetString("name") <<
"_";
- name << "port";
-
- reply.AddInt32(name.String(),
- iterator->second.GetInt32("port", -1));
- }
-
// Launch the job if it hasn't been launched already
if (launchJob)
_LaunchJob(job);
+
+ DetachCurrentMessage();
+ status_t result = job->HandleGetLaunchData(message);
+ if (result == B_OK) {
+ // Replying is delegated to the job.
+ return;
+ }
+
+ ownsMessage = true;
+ reply.what = result;
}
+
message->SendReply(&reply);
+ if (ownsMessage)
+ delete message;
}




Other related posts: