8388786: [macOS] NSImage leak in GlassPasteboard.m when using images for dnd or clipboard operations

[jperedadnr]

This PR releases the NSImage that SetNSPasteboardItemValueForUtf() in GlassPasteboard.m obtains from getImage() in GlassPixels.m. Once the pixel data is used, such image is not longer needed and has to be released, preventing a memory leak.

Similar operations in GlassCursor.m and GlassMenu.m that use the same NSImage allocation mechanism, did already release the image in this very same way, so this PR just adds the missing release call to GlassPasteboard.m.

There are no tests included, since this is a pure native objective-c memory leak (retained NSImages), and it can't be tracked down from the Java side, but I have tested manually with Instruments and the Leaks template, with this code snippet in MacPasteboardTest:

private void pushImage() {
        int size = 512; 
        ByteBuffer buffer = ByteBuffer.allocate(size * size * 4);
        Pixels pixels = Application.GetApplication().createPixels(size, size, buffer);
        macPasteboardShim.pushMacPasteboard(new HashMap<>(Map.of(Clipboard.RAW_IMAGE_TYPE, pixels)));
    }

calling it repeatedly for some time.

Before the fix:

as shown, the leaks are in getImage from libglass.dylib, that allocates a NSImage with a CGImage (including same amount of internal data, providers, arrays, snapshots objects).

After the fix the leaks are gone:

---

• I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8388786: [macOS] NSImage leak in GlassPasteboard.m when using images for dnd or clipboard operations (Bug - P4)

Reviewers

• Martin Fox (@beldenfox - Committer)
• Kevin Rushforth (@kevinrushforth - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/2222/head:pull/2222
$ git checkout pull/2222

Update a local copy of the PR:
$ git checkout pull/2222
$ git pull https://git.openjdk.org/jfx.git pull/2222/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2222

View PR using the GUI difftool:
$ git pr show -t 2222

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/2222.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jpereda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@jperedadnr This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8388786: [macOS] NSImage leak in GlassPasteboard.m when using images for dnd or clipboard operations

Reviewed-by: mfox, kcr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 5 new commits pushed to the master branch:

• 05a7b6d: 8388526: [Windows] Crash in Platform.Preferences refresh on OS theme change when using FXCanvas
• 038d790: 8389371: cssref: explain font properties
• ad6c0a0: 8389093: Typos in the JavaFX CSS Reference Guide
• ... and 2 more: https://git.openjdk.org/jfx/compare/1e7e7d03e85f9aa3bca97f4e6ec64deaf9c0bdf6...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

The total number of required reviews for this PR has been set to 2 based on the presence of this label: rfr. This can be overridden with the /reviewers command.

[mlbridge]

Webrevs

• 00: Full (d0798bf9)

[beldenfox]

I verified that PixelsAttachData creates the NSImage using alloc so it needs to be manually released and other clients of this API do call release. Looks good to me.

[kevinrushforth]

This looks like a correct fix to me. NSImage::TIFFRepresentation returns a new image and doesn't take ownership of the NSImage, so releasing it is the right thing to do.