Status

()

Core
XPCOM
--
major
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Colin Blake, Assigned: Colin Blake)

Tracking

({fixed1.8})

Trunk
x86
Windows XP
fixed1.8
Points:
---
Bug Flags:
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

12 years ago
Threads are getting wasted, footprint like this:

ntdll!KiFastSystemCallRet
ntdll!ZwWaitForSingleObject+0xc
kernel32!WaitForSingleObjectEx+0xa8
kernel32!WaitForSingleObject+0x12
nspr4!_PR_MD_WAIT+0x2c
nspr4!_PR_NativeRunThread+0x168 -> _PR_NotifyJoinWaiters(thread);
nspr4!pr_root+0xb
MSVCR71!_endthreadex+0xa0
kernel32!BaseThreadStart+0x37

The fix for bug 296506 was a little too zealous. mThread is getting set
to null and this is preventing join from doing its thing.

Patch coming...
(Assignee)

Comment 1

12 years ago
Created attachment 197891 [details] [diff] [review]
Patch for trunk
(Assignee)

Updated

12 years ago
Attachment #197891 - Flags: superreview?(darin)
Attachment #197891 - Flags: review?(timeless)

Updated

12 years ago
Attachment #197891 - Flags: review?(timeless) → review+
(Assignee)

Updated

12 years ago
Assignee: dougt → colin
Looks like we should take this for 1.8, right?
Blocks: 296505
Flags: blocking1.8b5?

Comment 3

12 years ago
It looks like this makes the fix for bug 308404 unnecessary.  Should that be
backed out now?  If so, then please cut a new patch.
(Assignee)

Comment 4

12 years ago
Talked with timeless and while the fix for bug 308404 could most likely be
backed out, there would then be some other work needed. The safest solution for
now is to take this one line fix and leave the fix for 308404 in.

Updated

12 years ago
Flags: blocking1.8b5? → blocking1.8b5+

Comment 5

12 years ago
Comment on attachment 197891 [details] [diff] [review]
Patch for trunk

I don't see why there is any risk to backing out the previous patch once this
change goes in.  How about making the correct change on the trunk at least?  I
can understand being more conservative on the 1.8 branch (even though it's
overly conservative in my opinion).

sr=darin
Attachment #197891 - Flags: superreview?(darin) → superreview+

Updated

12 years ago
Attachment #197891 - Flags: approval1.8b5?

Comment 6

12 years ago
Comment on attachment 197891 [details] [diff] [review]
Patch for trunk

Can you get this checked in on the branch today Colin? If not, we'll try to
find someone who can for you.
Attachment #197891 - Flags: approval1.8b5? → approval1.8b5+
(Assignee)

Comment 7

12 years ago
I don't recall my cvs login information, it's been a while. I'll submit the 1.8
patch if you can check them both in please.
(Assignee)

Comment 8

12 years ago
Created attachment 198069 [details] [diff] [review]
Patch for  MOZILLA_1_8_BRANCH

Updated

12 years ago
Whiteboard: [has patch, needs checkin]

Comment 9

12 years ago
fixed branch and trunk. 
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Whiteboard: [has patch, needs checkin]
You need to log in before you can comment on or make changes to this bug.