Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#1162 closed bug (fixed)

B_SILENT_RELAUNCH implemented but doesn't get sent?

Reported by: jonas.kirilla Owned by: bonefish
Priority: normal Milestone: R1
Component: Kits/Application Kit Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

Most of the implementation of B_SILENT_RELAUNCH appears to be there already. If the following code is added, applications do get B_SILENT_RELAUNCH messages, and windows get focus, even without catching B_SILENT_RELAUNCH in MessageReceived().

In src/kits/app/Roster.cpp, BRoster::_LaunchApp(), near the end, add this:

if (alreadyRunning) {

BList list; list.AddItem(new BMessage(B_SILENT_RELAUNCH)); _SendToRunning(team, 0, NULL, &list, NULL, false);

}

This however, appears to be contrary to the current design. There is mention of B_SILENT_RELAUNCH in src/kits/app/Application.cpp, BApplication::_InitData().

...

fInitError = BRoster::Private().AddApplication(signature, &ref, appFlags, team, thread, fMsgPort, true, NULL, &otherTeam);

...

if (fInitError == B_ALREADY_RUNNING) {

An instance is already running and we asked for single/exclusive launch. Send our argv to the running app. Do that only, if the app is NOT B_ARGV_ONLY. if (otherTeam >= 0) {

BMessenger otherApp(NULL, otherTeam); app_info otherAppInfo; if (libc_argc > 1

... otherApp.SendMessage(&argvMessage);

} else

otherApp.SendMessage(B_SILENT_RELAUNCH);

}

}

Perhaps the B_ALREADY_RUNNING isn't returned, but it seems a reply does get sent back from Registrar's TRoster::HandleAddApplication(BMessage *request). I dunno.

I suppose it's best to fix the current implementation and let BApplication do B_SILENT_RELAUNCH, but on the other hand, it would be more efficient to have the BRoster::Launch() variations /also?/ stop the buck, so disk, memory and processor resources aren't spent needlessly.

Change History (6)

comment:1 by axeld, 12 years ago

Owner: changed from axeld to bonefish

Maybe Ingo want to have a look at it.

comment:2 by bonefish, 12 years ago

Resolution: fixed
Status: newclosed

Fixed in hrev20743. BRoster::Launch() (BRoster::_SendToRunning() in actual fact) sends the B_SILENT_RELAUNCH message now, if required (i.e. the app isn't B_ARGV_ONLY and no args have been given).

This isn't contrary to the design. There are simply two different methods for starting an application. One is using the low-level functions exec() or load_image(), and one is using BRoster::Launch(). The former requires the BApplication constructor to deal with single/exclusive launch handling and sending the respective messages to an already running app. BRoster::Launch() uses load_image() in the end, which would do the job alright, but since it has to provide special features (e.g. sending entry_refs or a list of given messages, and returning B_ALREADY_RUNNING, if the app was already running) it needs to implement a similar handling. With the advantage that it "stops the buck", as you put it; i.e. a running B_SINGLE_LAUNCH app is never relaunched via BRoster::Launch().

comment:3 by jonas.kirilla, 12 years ago

Looking at the apps' behaviour B_SILENT_RELAUNCH appears to be working now. Thank you!

But the code in BApplication must be wrong somehow, if it was indeed intended to catch duplicates on both exec()/load_image() and BRoster::Launch() methods. The BApplication constructor is run of course, and even so it seems no B_SILENT_RELAUNCH was received by any of the Haiku apps included. The BRoster fix may conceal that bug in BApplication, if it is a bug.

in reply to:  3 ; comment:4 by bonefish, 12 years ago

Replying to jonas.kirilla:

But the code in BApplication must be wrong somehow, if it was indeed intended to catch duplicates on both exec()/load_image() and BRoster::Launch() methods. The BApplication constructor is run of course, and even so it seems no B_SILENT_RELAUNCH was received by any of the Haiku apps included. The BRoster fix may conceal that bug in BApplication, if it is a bug.

Not sure what duplicates you're referring to. The BApplication "already running" code is executed when the app is started e.g. via the shell (fork()+exec()). If someone requests to start an already running (single/exclusive launch) app via BRoster::Launch() the app is never started, and thus obviously the BApplication constructor is never executed either.

in reply to:  4 ; comment:5 by jonas.kirilla, 12 years ago

Replying to bonefish:

Not sure what duplicates you're referring to. The BApplication "already running" code is executed when the app is started e.g. via the shell (fork()+exec()). If someone requests to start an already running (single/exclusive launch) app via BRoster::Launch() the app is never started, and thus obviously the BApplication constructor is never executed either.

By duplicates I meant "attempts to launch multiple instances of a single/exclusive-launch application". I meant that if the BApplication::_InitData is meant to be a catch-all, I would have expected the B_SILENT_RELAUNCH code that is there, to also work for attempts to launch additional instances via BRoster::Launch(), (prior to the fix), as the BApplication constructor is always called, (unless you can fork a BApplication these days).

Prior to the fix, unless I'm completely mistaken, the B_SILENT_RELAUNCH didn't get sent, for apps launched via BRoster. (At least observation of the apps launched from Tracker hints to that there was none received.)

As both ways to try to launch additional instances of a single/exclusive-launch app have checks added now, perhaps it doesn't matter that much. Anyway, I'm not going to pursue this any further.

in reply to:  5 comment:6 by bonefish, 12 years ago

Replying to jonas.kirilla:

By duplicates I meant "attempts to launch multiple instances of a single/exclusive-launch application". I meant that if the BApplication::_InitData is meant to be a catch-all, I would have expected the B_SILENT_RELAUNCH code that is there, to also work for attempts to launch additional instances via BRoster::Launch(), (prior to the fix), as the BApplication constructor is always called, (unless you can fork a BApplication these days).

This is a misperception. The BApplication isn't a catch-all, it's a "catch the launches we cannot otherwise control", i.e. those directly done via exec()/load_image(). BRoster::Launch() avoids loading an already running single/exclusive launch application.

Prior to the fix, unless I'm completely mistaken, the B_SILENT_RELAUNCH didn't get sent, for apps launched via BRoster. (At least observation of the apps launched from Tracker hints to that there was none received.)

Yep.

As both ways to try to launch additional instances of a single/exclusive-launch app have checks added now, perhaps it doesn't matter that much.

It simply cannot work any other way. We have to do it in BRoster::Launch() due to the API requirements. We also have to do it in the BApplication constructor, since we do not control the launching behavior of exec()/load_image(). We could in fact, but the overhead (checking executable attributes and resources, contacting the registrar) would probably degrade shell scripting performance.

Anyway, I'm not going to pursue this any further.

In case you want to, a mail to an appropriate list or to me privately would be the best choice. :-)

Note: See TracTickets for help on using tickets.