On Tue, 21 Jul 2026 17:45:53 GMT, Martin Fox <[email protected]> wrote:
>> This PR creates a native owned copy of the pixel buffer passed to >> GlassPixels::getImage, to let AppKit start, at any later time, the >> rasterization of the images for the system menu bars with a native object >> that outlives the Java buffer. Therefore, if under certain conditions of >> high memory pressure, the latter is reclaimed and collected, the former is >> still available. >> >> A manual test has been included. >> >> Since this is a memory pressure/timing issue, it is not deterministic, and >> depends on several conditions. Therefore, it needs to be run several times >> to spot the issue (before the proposed fix). After the fix, the issue should >> never happen again. >> >> When running it from `tests/manual/systemmenu`, with: >> >> $JAVA_HOME/bin/java @../../../build/run.args >> MacOSSystemMenuImageGraphicTest.java >> >> it might not trigger the crash, but still the icons might be missing or >> garbled most of the runs. >> >> However, with: >> >> >> $JAVA_HOME/bin/javac @../../../build/run.args >> MacOSSystemMenuImageGraphicTest.java // remove --enable-native-access from >> run.args >> $JAVA_HOME/bin/java @../../../build/run.args MacOSSystemMenuImageGraphicTest >> >> the crash is more likely to happen in different runs. >> >> Other options to increase the likelihood of failure is to use a low Xmx, or >> to increase the `COUNTER` value. >> >> --------- >> - [X] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > I could easily reproduce the bug in the master branch and this PR fixes it. > Linux and Windows also copy the data (it's deferred on Windows but it does > get copied). The reference counting in this PR looks correct and overall the > code looks good. > > I think there's an existing bug which leaks one of these NSImages. In > GlassPasteboard.m look for jPixelsAttachData (currently on line 270). The > NSImage created there is not released and since it's created using `alloc` it > won't be autoreleased either. The two other calls to jPixelsAttachData are > releasing their NSImages. Thanks @beldenfox. I do see the leak in GlassPasteboard, which might be related to the fix proposed in this PR for `GlassPixels::getImage`. As you say, GlassPasteboard creates a `NSImage` that is allocated in `GlassPixels::getImage` but never released (while GlassCursor and GlassMenu do release it), but I fail to see this as related to the same bug/JBS ticket. While the fix for this leak would be really small (just adding `[image release];` to GlassPasteboard), I'm not sure it should be part of this PR? Shouldn't we file a separated JBS ticket for it instead? ------------- PR Comment: https://git.openjdk.org/jfx/pull/2219#issuecomment-5037788168
