Last Comment Bug 713068 - IonMonkey: Invalidation leaks IonScripts
: IonMonkey: Invalidation leaks IonScripts
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: David Anderson [:dvander]
:
Mentors:
Depends on:
Blocks: IonOSI
  Show dependency treegraph
 
Reported: 2011-12-22 12:30 PST by David Anderson [:dvander]
Modified: 2011-12-27 17:57 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (8.96 KB, patch)
2011-12-22 18:07 PST, David Anderson [:dvander]
cdleary: review+
Details | Diff | Splinter Review

Description David Anderson [:dvander] 2011-12-22 12:30:57 PST
IonScripts are not gcthings, so when we invalidate, and detach them from JSScripts, they don't get freed later. Simple solution is to just refcount them.
Comment 1 Chris Leary [:cdleary] (not checking bugmail) 2011-12-22 14:05:27 PST
Is this instead of the HandleException walk-and-free approach? I'm just curious because performing decref operations appropriately doesn't seem inherently easier than performing a free operation appropriately.
Comment 2 David Anderson [:dvander] 2011-12-22 14:13:58 PST
Nah they're orthogonal. We'd incref the IonScript when we create an InvalidationRecord for it, and decref the IonScript when we destroy the record. The HandleException approach is fine we just have to destroy the IonScripts too.
Comment 3 Chris Leary [:cdleary] (not checking bugmail) 2011-12-22 14:58:58 PST
Oh right. At least that's nice and limited in scope -- don't need to worry about keeping a refcount until it actually becomes invalidated.
Comment 4 David Anderson [:dvander] 2011-12-22 18:07:51 PST
Created attachment 583982 [details] [diff] [review]
fix
Comment 5 Chris Leary [:cdleary] (not checking bugmail) 2011-12-22 22:03:56 PST
Comment on attachment 583982 [details] [diff] [review]
fix

Review of attachment 583982 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ion/IonFrames.h
@@ +166,5 @@
>      void *calleeToken;
>      uint8 *returnAddress;
>      IonScript *ionScript;
>  
>      InvalidationRecord(void *calleeToken, uint8 *returnAddress);

Should we privatize this stuff now?
Comment 6 David Anderson [:dvander] 2011-12-27 17:57:59 PST
I tried :( unfortunately new_ requires public visibility.

http://hg.mozilla.org/projects/ionmonkey/rev/83f981a4e266

Note You need to log in before you can comment on or make changes to this bug.