340889 - Hang on shutdown dealing with close hooks

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Blake Kaplan (:mrbkap) (inactive)]

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.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: 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?

Comment 2

[Igor Bukanov]

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.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: 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.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

I filed bug 340899 on the leaks.