Hang on shutdown dealing with close hooks

RESOLVED FIXED in mozilla1.9alpha1

Status

()

P1
normal
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: mrbkap, Assigned: mrbkap)

Tracking

({hang})

Trunk
mozilla1.9alpha1
x86
Linux
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

I've been noticing for the past couple of weeks a hang (or pseudo hang, it actually does finish eventually, soemtimes) on shutdown. I finally got it in a debugger today and found a probable cause:

Currently, if we're on the last context and there's more than zero object with a close hook, then we'll set rt->gcPoke to JS_TRUE unconditionally to allow any garbage generated by the close hooks to be collected on the next run. However, if we've already marked all of the objects in the close array, then we'll not call any close hooks, not free any objects, and loop unnecessarily. I think this can only happen with leaked iterators, but I don't think that leaks should cause the GC to hang.
Created attachment 224930 [details] [diff] [review]
Possible fix -w

This is a potential patch. If |count != pivot| then that means that we will not call any close hooks (since all of the objects in the close list were marked already), so we can avoid doing any work at all.

This is a diff -w. The real patch will have to include some rewrapping, but I left that out for clarity.

Igor, what do you think?
Attachment #224930 - Flags: review?(igor.bukanov)
Status: NEW → ASSIGNED
Priority: -- → P1

Comment 2

13 years ago
Comment on attachment 224930 [details] [diff] [review]
Possible fix -w

If on shutdown rt->gcCloseTable stays above 0, then it must be a leak that keeps some iterators alive. It can be worth to investigate what exactly marks the iterators but for now lets fix this real bug.
Attachment #224930 - Flags: review?(igor.bukanov) → review+
Created attachment 224940 [details] [diff] [review]
Patch I'm about to check in

I'm going to check this in. Igor, if you spot any style problems with this, then go ahead and check them in.
Attachment #224940 - Flags: review?(igor.bukanov)
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
I filed bug 340899 on the leaks.

Updated

13 years ago
Attachment #224940 - Flags: review?(igor.bukanov) → review+

Updated

12 years ago
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.