1008271 - JNI.jsm leaks references

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

JNI.jsm leaks local references when calling FindClass/GetGlobalClassRef, leading to crashes like this one, https://crash-stats.mozilla.com/report/index/cacf2a86-be4f-48b8-ba89-4d9872140508 (bug 825996)

We can fix the leaks, or go ahead with bug 918309 (with some cleanup).

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Mark, what do you think?

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

I'd like to try and fix the leaks without needing bug 918309, since that adds additional risk.

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Fix class reference leak in JNI.jsm (v1)

This patch fixes the leak by remembering global references and deleting them on close.

Comment 4

[Wesley Johnston (:wesj)]

Comment on attachment 8422495 [details] [diff] [review]
Fix class reference leak in JNI.jsm (v1)

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

I'll review this if you want, but I talked to finkle last week and I think we're up for just using the public domain JNI.jsm from bug 918309. It handles this on its own and lets us kill some of our un-needed C++ as well. I think I'd rather that than expose more C++, but we may have to do some small changes to our existing JNI.jsm usage. I'd be fine bringing it in with little/no changes and then fixing things in it as we found them. We can update the code style to be more Fennec like at a later time (or leave it and just treat it as a third party library).

What do you think jchen?

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

I think it's up for the front-end team to decide since they're the most likely users :)  But I do think we'd be better off using the full JNI.jsm.

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

Closing as WONTFIX because we want bug 918309 instead. I don't think this crash is frequent enough to warrant an urgent fix ATM.