RFR: 8088420: JavaFX WebView memory leak via EventListener [v7]
Jay Bhaskar
jbhaskar at openjdk.java.net
Fri May 27 12:13:46 UTC 2022
On Thu, 26 May 2022 17:00:46 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>> Jay Bhaskar has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Adding space for map include
>
> modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.cpp line 106:
>
>> 104: else
>> 105: isReferringToOtherListener = true;
>> 106: }
>
> I think I now understand what this is trying to do, but it doesn't looks like it's implemented correctly nor is it optimal.
>
> It seems that the `isReferringToOtherListener` flag is intended to be true iff there is any `JavaEventListener` with a ref count > 1 associated with the Window being unregistered. Ignoring any possible bugs in how it is implemented, there are two problems with this approach. First, you want to remove entries associated with a particular listener if _that_ listener isn't used by another window. So a global "is any listener referring to another window" isn't what you want. Second, since this is multimap, it would seem better to remove individual `(key,value)` pairs associated with the specific window being removed rather than calling erase only for those listeners that aren't being shared at all. If this is feasible, then you wouldn't have to even care if that listener is used by a node in another window. I'm not familiar enough with C++ multimap calls to know how easy this would be, but presumably, it shouldn't be too hard.
>
> Not directly related to this, it might be cleaner to move this logic to `unregisterDOMWindow` instead of having a separate `resetDOMWindow` method, since this is really part of the same conceptual operation.
I have used the suggested method , and tested it works expected. Thanks
-------------
PR: https://git.openjdk.java.net/jfx/pull/799
More information about the openjfx-dev
mailing list