713068 - IonMonkey: Invalidation leaks IonScripts

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

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)]

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] - inactive, e-mail if emergency]

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)]

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] - inactive, e-mail if emergency]

Attachment: fix

Comment 5

[Chris Leary [:cdleary] (not checking bugmail)]

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] - inactive, e-mail if emergency]

I tried :( unfortunately new_ requires public visibility.

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