Closed Bug 444633 Opened 13 years ago Closed 3 years ago

[BEOS]Message pump in AppShell relies on bad assumption

Categories

(Core Graveyard :: Widget: BeOS, defect)

x86
BeOS
defect
Not set
major

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: sergei_d, Assigned: sergei_d)

Details

Attachments

(2 files)

for internal communication we use BeOS ports, which are OS-global objects, but assign to those names generated from PR_GetCurrentThread - actually from pointer, which value isn't OS-wide unique.

That worked with BeOS memory allocator, by occasion, actually, but in Haiku-OS those pointers get same value for simultaneously running Mozilla applications.

Which leads to closing one app by another.

We need to replace that bad port name generation in nsAppShell, nsToolkit and in plevent

http://community.livejournal.com/bezilla/282356.html

actual for all branches and trunks, but we do care only about 1.8 and above
Attached patch patchSplinter Review
using find_thread(appname) everywhere to generate ports and semaphores names.

looks like it is sufficient workaround for 1.8
Assignee: nobody → sergei_d
Status: NEW → ASSIGNED
Attachment #329080 - Flags: review?(thesuckiestemail)
changing to MAJOR, as OS-global objects objects like port and semaphores may damage other apps or even OS when used unsafely
Severity: normal → major
Attached patch uses strrcharSplinter Review
same as previous, but uniformely uses strrchar instead BPath::Leaf() everywhere, not only in plevent.c

may be bit faster than previous, but that's for choice by taste
after little tests it looks like second version is really bit faster
I've built Firefox and Thunderbird under Zeta using 2nd version.  Will build both under R5+BONE and test with Haiku this morning and report.
Built Firefox and Thunderbird under BeOS BONE using second version.  Tested under Haiku and seems to work fine.  Nice work, fyysik!
fyysik and tqh, how close are we to committing this patch?  Do you need further testing?
(In reply to comment #7)
> fyysik and tqh, how close are we to committing this patch?  Do you need further
> testing?
> 
It's up to tqh. Both patches work OK for me since introduction, but personally I prefer second one. If tqh has neutral position and lack of time, I can put review request for you for second patch.
Comment on attachment 329115 [details] [diff] [review]
uses strrchar

Been using this version for months in Firefox and Thunderbird.  Please review so we can commit.
Attachment #329115 - Flags: review?(thesuckiestemail)
Sorry I've must have missed the patches completly. Will look at it tomorrow.
Comment on attachment 329080 [details] [diff] [review]
patch

r-
The assumption that get_next_image_info(0, &cookie, &iinfo) is the app image may no longer be true with Haiku, although Ingo fixed that for now.

Couldn't we use arg[0] instead?
Attachment #329080 - Flags: review?(thesuckiestemail) → review-
Comment on attachment 329115 [details] [diff] [review]
uses strrchar

r-
See comments on other patch.
Attachment #329115 - Flags: review?(thesuckiestemail) → review-
> The assumption that get_next_image_info(0, &cookie, &iinfo) is the app image
> may no longer be true with Haiku, although Ingo fixed that for now.

Why so?

> Couldn't we use arg[0] instead?
That was my first idea. But is argc[0] accessible globally, outside of scope of main? I wish we have something like be_app->Name().
tqh, can you suggest an alernative workaround?
Sergei, might be available by tinfo.args
after
  get_team_info(0, &tinfo);

Need to verify this is true launching by commandline full/relative path and by click launching firefox-bin.

Otherwise loop thru the image until the type is of app image type, which is only small addition to the current code.

(Note I don't seem to get Mozilla mails atm. So I may miss a lot.)
Hmm, yahoo filters mozilla mails as spam :(
get_team_info(0, &tinfo) suffers from the following:
only 64 bytes and it also includes args as one string.

So not such a good idea.
(In reply to comment #16)
> Hmm, yahoo filters mozilla mails as spam :(

Your name says it all:  "the suckiest email" :)
Sergei I think we can just get the team id instead of trying to find_thread's.
plevent.c would then be something like:
    team_info tinfo;
    get_team_info(0, &tinfo);
    
    PR_snprintf(portname, sizeof(portname), "MozEvent%lx", tinfo.team);
    PR_snprintf(semname, sizeof(semname), "MozSync%lx", tinfo.team);
sergei and tqh, can we please get an update on this?  Haiku really needs a fix for this issue to be in the branch before Haiku Alpha release.  I'd be happy to try tqh's suggested code, but I don't trust myself to put it in the right place.  sergei, what do you think?
(In reply to comment #21)
> sergei and tqh, can we please get an update on this?  Haiku really needs a fix
> for this issue to be in the branch before Haiku Alpha release.  I'd be happy to
> try tqh's suggested code, but I don't trust myself to put it in the right
> place.  sergei, what do you think?

Will be really busy next two days at least.
Is this bug still relevant?
Product: Core → Core Graveyard
Nothing has been happening on the beos side for a while, closing.
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.