On Tue, 15 Jul 2025 21:43:04 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>> src/java.desktop/macosx/classes/apple/laf/JRSUIControl.java line 115:
>>
>>> 113: flipped = other.flipped;
>>> 114: cfDictionaryPtr = getCFDictionary(flipped);
>>> 115: if (cfDictionaryPtr == 0) throw new RuntimeException("Unable
>>> to create native representation");
>>
>> Just a side topic to discuss: In the past I tried to look into this pattern
>> and figure out is it possible for the GC to start cleaning the object after
>> we allocate the native resource but before we register it with
>> Disposer.addRecord. In such a case, the cleanup might never be executed.
>>
>> Perhaps we should consider adding a reachabilityFence at the end of
>> Disposer.addRecord, for both the DisposerRecord and the referent, to ensure
>> they remain reachable. Similarly how it was done for cleaner:
>> https://hg.openjdk.org/jdk9/jdk9/jdk/rev/1f33e517236e
>> Or may be we can just use cleaner.
>
> there is even a bug for Cleaner that the reachabilityFence should be moved to
> the end of register:
> https://bugs.openjdk.org/browse/JDK-8291867
> If there are no objectiosn I'll add it to the Disposer.addRecord in a
> separate bug.
I think that would be OK, meaning probably harmless. But I don't know if this
case actually requires it.
Where I've seen a problem like this a Java object is used at an earlier point
in the method and not again and so becomes eligible to be freed, but some
native resource it held is needed later but is already freed along with its no
longer references holder.
Here, if this object (the JRSUIControl) becomes unreachable during
construction, and so the referent and disposer are also unreachable outside
this object, they are still going to be live until we reach code in the
constructor which uses them and since the referent and the disposer are passed
into addRecord they'll be live there until stored.
But these cases are subtle, maybe I'm missing something. Can you expand on the
scenario ?
Or maybe there's some other patterns where we use Disposer that it is needed ?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26331#discussion_r2208837967
