RFR: 8319779: SystemMenu: memory leak due to listener never being removed [v10]

Johan Vos jvos at openjdk.org
Fri Mar 1 14:59:02 UTC 2024


On Tue, 27 Feb 2024 09:30:14 GMT, Johan Vos <jvos at openjdk.org> wrote:

>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
> 
>  - Merge branch 'master' into 8319779-systemmenu
>  - Add additional test for IOOBE detection.
>    This test comes from JDK-8323787
>  - Revert some of the conditional bindings.
>    Clear menu construction when an menuitem that is a menu needs to be removed
>    Add a test for the latter
>  - Merge remote-tracking branch 'upstream/master' into 8319779-systemmenu
>  - Cleanup test
>  - Add shim class so that we can access the references to com.sun.glass.ui.Menu instances.
>    Add a test to make sure those references are gone.
>  - Revert WeakInvalidationListeners and use new listener resource management approach.
>  - Fix more memoryleaks due to listeners never being unregistered.
>  - These changes are related to JBS-8318841 so we want to have that code in
>    as well.
>    
>    Merge branch 'master' into 8319779-systemmenu
>  - process reviewers comments
>  - ... and 1 more: https://git.openjdk.org/jfx/compare/831d6526...ec7308df

The failure on Linux has an interesting cause. The GlassSystemMenu (in com.sun.javafx.tk.quantum) has this code:

            /*
             * Leave the Apple menu in place
             */
            for (int index = existingSize - 1; index >= 0; index--) {
                Menu menu = existingMenus.get(index);
                clearMenu(menu);
                glassSystemMenuBar.remove(index);
            }

As a consequence, the first menu is not cleared -- regardless of the presence of an apple or not.
There are plenty quick and dirty ways to fix this, but this platform-specific check probably does not belong in this package.
@kevinrushforth what do you prefer to do in cases like these?

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1283#issuecomment-1973342702


More information about the openjfx-dev mailing list