Closed Bug 310487 Opened 15 years ago Closed 15 years ago

thread leak

Categories

(Core :: XPCOM, defect)

x86
Windows XP
defect
Not set
major

Tracking

()

RESOLVED FIXED

People

(Reporter: colin, Assigned: colin)

References

Details

(Keywords: fixed1.8)

Attachments

(2 files)

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...
Attached patch Patch for trunkSplinter Review
Attachment #197891 - Flags: superreview?(darin)
Attachment #197891 - Flags: review?(timeless)
Attachment #197891 - Flags: review?(timeless) → review+
Assignee: dougt → colin
Looks like we should take this for 1.8, right?
Blocks: 296505
Flags: blocking1.8b5?
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.
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.
Flags: blocking1.8b5? → blocking1.8b5+
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+
Attachment #197891 - Flags: approval1.8b5?
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+
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.
Whiteboard: [has patch, needs checkin]
fixed branch and trunk. 
Status: NEW → RESOLVED
Closed: 15 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.