RFR: JDK-8318854: [macos14] Running any AWT app prints Secure coding warning [v2]

[Harshitha Onkar]

On Fri, 10 Nov 2023 22:37:42 GMT, Phil Race <prr at openjdk.org> wrote:

>> In the case when AWT is embedded AWT does NOT start an  NSApplication and does not own it.
>> AWT overwriting FX's delegate, or any one else's is just plain wrong.
>> 
>> We can turn around what you said
>> "To access any native code in JavafX, the app should initialise GlassToolkit which sets its delegate, if not already set".
>> 
>> But you can't have both of these.
>> 
>> Meaning AWT isn't special here. It has to play by the same rules.
>> 
>> The toolkit that starts the NSApplication is the one that should be setting the delegate.
>> If you are embedded in someone else's application, it seems most logical that you defer to their application
>> and if this means some things aren't possible,  so be it.
>
>> Can you please provide details on how the delegate can be null here and there if it should be set at the startup of any awt application?
> 
> I commented on these in https://bugs.openjdk.org/browse/JDK-8319255 which is now closed as a dup.
> There I wrote (in part) 
> #######
> 
> I am not sure if these are all cases that can provably be in un-reachable code in the "nil" case, which
> means some embedded case. But if we ever do reach them, it seems we'd crash the app.
> And if we need to make the delegate "nil" in the non-subclassed NSApplication case (ie like FX)
> then we are more likely to cause a crash.
> 
> [list of places elided]
> 
> All these call sites need to be examined to understand that, and if necessary a test developed to
> confirm it.
> And even then, it might be prudent to add checks for nil.
> #####
> 
> So the request was to investigate and understand if they were needed, not just add them
> @honkar-jdk can comment on what she did there.

@prrace @kevinrushforth @mrserb 

The reason has to why we don't observe NPE or crash when sharedDelegate is null is because in Objective C when a method is called on a nil object it is treated as no-op and returns nil instead of NPE, contrary to what we would usually expect. Details [here](https://stackoverflow.com/questions/2696891/calling-a-method-on-an-uninitialized-object-null-pointer).

For example executing the following line when shared delegate is null and accessing defaultMenuBar on null delegate returns null and not NPE.

`[[ApplicationDelegate sharedDelegate] defaultMenuBar];`

Additionally we are not seeing any difference in defaultMenuBar with unpatched JDK + unpatched JFX (Non-Nil shared delegate) vs patched JDK + patched JFX (Nil shared delegate) at [AWTWindow.m](https://github.com/openjdk/jdk/blob/5493f5f92d569d8e94d1f271480f11c48257e896/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m#L844) and [CMenubar.m](https://github.com/openjdk/jdk/blob/5493f5f92d569d8e94d1f271480f11c48257e896/src/java.desktop/macosx/native/libawt_lwawt/awt/CMenuBar.m#L213)  because in both cases defaultMenuBar returned is nil. This might be the case at other places too and are being checked if they worked differently without and with this JDK patch.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16569#discussion_r1399604866