js_FinishDeflatedStringCache doesn't free the strings owned by rt->deflatedStringCache; it only frees the table. This shows up as a rather large number of shutdown leaks (pretty easy to spot now that trace-malloc runs on Windows). It seems like the best fix would be to use a non-default set of JSHashAllocOps (although it's a bit of a pain for overriding just one of them) and then make js_PurgeDeflatedStringCache not call free manually (and probably just use Remove rather than RawRemove).
So, brendan pointed out that what I said above shouldn't happen because the strings should have all been freed. The problem is the order in which we call js_FinishDeflatedStringCache and js_FinishAtomState; js_FinishAtomState finalizes strings, so we shouldn't finish the deflated string cache until after it has run. Patch coming shortly, after testing that it fixes the leak.
Assignee: general → dbaron
Summary: js_FinishDeflatedStringCache leaks the strings in the cache on shutdown → js_FinishDeflatedStringCache leaks the strings for interned atoms on shutdown
Target Milestone: --- → mozilla1.9 M8
Created attachment 276060 [details] [diff] [review] patch
Attachment #276060 - Flags: review?(brendan)
Comment on attachment 276060 [details] [diff] [review] patch Nit: one blank line before a comment that takes up one or more whole lines. Thanks for patching! /be
Attachment #276060 - Flags: review?(brendan) → review+
Comment on attachment 276060 [details] [diff] [review] patch Should I add a blank line after as well?
Attachment #276060 - Flags: approval1.9?
Comment on attachment 276060 [details] [diff] [review] patch Sure, after too is ok, not strictly necessary either. /be
Attachment #276060 - Flags: approval1.9? → approval1.9+
Fix checked in to trunk, 2007-08-10 14:30.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.