282031 - [BEOS] Our nsToolkit::CallMethod is broken

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[tqh]

User-Agent:       Mozilla/5.0 (BeOS; U; BeOS BePC; en-US; rv:1.8b) Gecko/20050212 Firefox/1.0+
Build Identifier: 

nsAppShell and nsToolkit uses a sync-semaphore to halt and wake up threads that
wants to do work in the GUI-thread. This is slow and there is no guarantee the
thread whose work was just processed is the one woken up.

Instead I suggest we pass the thread id in the call and then suspend the thread.
When the work is processed the GUI-thread wakes it up with a resume_thread.

Patch in progress.

Reproducible: Always

Comment 1

[tqh]

Attachment: Old patch, until I've made the real one. Should work though.

Comment 2

[tqh]

Attachment: Cleaned up patch

Tried to clean it up a little.

Comment 3

[tqh]

Attachment: Spotted some formatting things.

r?

Comment 4

[tqh]

Some of the changes are in unreachable nsToolkit-code, but I have a feeling I'm
going to do some work there soon :)

Comment 5

[tqh]

Attachment: xpcom/threads/plevent.c did actually know about this too

Fixed the types in plevent.c

Comment 6

[Sergei Dolgov]

Comment on attachment 174150 [details] [diff] [review]
Spotted some formatting things.

Works great.
r=sergei_d

Comment 7

[tqh]

As there is a small change in xpcom now I'm switching to xpcom-component.

Comment 8

[tqh]

Comment on attachment 174151 [details] [diff] [review]
xpcom/threads/plevent.c did actually know about this too

Better put a review on the lines in xpcom as well. r?

Comment 9

[Sergei Dolgov]

tqh, putting plevent change in same patch will cause need for sr also for
widget/src/beos changes:)

Comment 10

[Sergei Dolgov]

Comment on attachment 174151 [details] [diff] [review]
xpcom/threads/plevent.c did actually know about this too

r=sergei_d

actually, same as previous.

Need someone's help for sr and checkin, as now patch touches xpcom

Comment 11

[tqh]

Should I seperate them?

Comment 12

[tqh]

Attachment: Changes outside BeOS-folder (plevent.c)

Ok, so to easier get SR we do this.

Comment 13

[tqh]

Comment on attachment 174150 [details] [diff] [review]
Spotted some formatting things.

We'll use this one.

Comment 14

[tqh]

Comment on attachment 174151 [details] [diff] [review]
xpcom/threads/plevent.c did actually know about this too

And not this one, they are identical, except for xpcom/threads/plevent.c which
is in last patch now.

Comment 15

[Sergei Dolgov]

Comment on attachment 174212 [details] [diff] [review]
Changes outside BeOS-folder (plevent.c)

r=sergei_d

Comment 16

[tqh]

Comment on attachment 174212 [details] [diff] [review]
Changes outside BeOS-folder (plevent.c)

sr?

(This is BeOS only changes in our nsAppShell / nsToolkit, but since plevent.c
knows about this as well we want that fixed. So we're doing sr on that part.)

Comment 17

[Doug Turner (:dougt)]

Comment on attachment 174212 [details] [diff] [review]
Changes outside BeOS-folder (plevent.c)

look fine.  need a checkin?

Comment 18

[tqh]

Yes, it would be great if you could checkin both patches, but isn't the tree
frozen for 1.8b?

Comment 19

[Doug Turner (:dougt)]

last patch checked in:

Checking in plevent.c;
/cvsroot/mozilla/xpcom/threads/plevent.c,v  <--  plevent.c
new revision: 1.50; previous revision: 1.49
done

Comment 20

[tqh]

Sergei, plevent.c is checked in, so could you check in our widget changes?

Comment 21

[Sergei Dolgov]

Do you mean this
https://bugzilla.mozilla.org/attachment.cgi?id=174150
one?

Comment 22

[tqh]

yes.

Comment 23

[Sergei Dolgov]

Done.
Checked in 2005-02-19 11:54 'bug 282031. Use suspend/resume_thread instead
syncsem for synchronous events. r=sergei_d. no sr required - BeOS-only'