Bug 1398

BTW, this occurs on MacOS Catalina (10.15).

Created attachment 824 [details]
Moves OpenGL context manipulations to main thread

Created attachment 825 [details]
Corrected patch to also work on macOS 10.8

On macOS 10.8 resources may get released prior to the code block executing on the main thread causing objc_msgSend to a deallocated instance.

Thank you. 
AFAIK I have already resolved this issue in one of my latest commits.
I will check this soon.
Earmarked for version 2.4.0 ofc.

Duplicate of Bug 1393

AFAIK fixes related to main-thread in Bug 1393 comment 2,
or commit b12a80e386b12d9d8fa63cf07124f8da989dcd04

    Bug 1393: Run orderFront0(=setVisible) async off-thread on AppKit after sync AppKit NSWindow creation
    
    MacOS 10.14.6 + OpenJDK11U produces occasional freezes on AppKit Main Thread
    
    Latest manual tests after resolving Bug 1389
    disclosed a few occasional freezes using NEWT + Java11.
    
    These are related to probable AWT changes since Java8,
    as these do not occur with Java8.
    
    Fix: Spun off orderFront0(=setVisible) async off-thread on AppKit after sync AppKit NSWindow creation.
    
    This fix also aligns the macos createWindow code with the new simplified ios implementation,
    see commit 004c67c73a0309158c30929cd0d6513e23f34803

commit e33aa16904d8abddaeceb1374ffa45bd45a96210

and

commit 7e76df3a05b7eb2404cb4584ee0b34ea287eb9bf (validation)

Please reopen 1393 (if I closed it and bugs persist)

*** This bug has been marked as a duplicate of bug 1393 ***

I have just reproduced this issue after pulling the latest master and typecasting all (except junit) position(int), limit​(int), flip​(), clear​(), mark​(), reset​(), and rewind​() NIO buffer calls I could find. I'm currently building with JDK9 but a need the software to run on JRE8 hence the typecast requirement.

Applying the attached patch fixes the issue.

(In reply to jani from comment #6)

It would help, if you could provide a unit test case
reproducing this issue.
As of today, this bug doesn't show up in our unit tests,
as they all pass on latest MacOS update.

Regardless, let me double check on your patch

1) NSOpenGLContext* createContext(NSOpenGLContext* share, NSView* view, ..
Java: CGL.createContext(long shareContext, long view, ..

[ctx setView:view]; -> main thread blocking

2) void setContextView(NSOpenGLContext* ctx, NSView* view)
Java: CGL.setContextView(long ctx, long view)

[ctx setView:view]; -> main thread blocking

3) void updateContext(NSOpenGLContext* ctx)
Java: CGL.updateContext(long ctx)

[ctx update]; -> main thread blocking

+++++++++++++++++++++++++++++++++++++++
+++++++++++++++++++++++++++++++++++++++

So this change would render all three method blocking until done,
potentially introducing a deadlock.
None of these methods were blocking before.

Now let me check their caller...

3) Java: CGL.updateContext(long ctx)

3.1) Caller 'jogamp.opengl.macosx.cgl.MacOSXCGLContext.drawableUpdatedNotify()'
I added a 'useAppKit' flag in 'jogamp.opengl.macosx.cgl.MacOSXCGLContext'
for testing months ago. 
Due to issues running on 'RunOnMainThread blocking', 
I have kept this to false.

I would need to retest also against your unit test.

Sideeffects to spawning of methods to the MainThread are always sensitive in nature, prone to cause a deadlock.

To provide a reliable resolution, I very often need to allow the caller to accept a delayed async transaction, i.e. not expecting the result yet and hence perform the action on MainThread _without_ blocking.

See evolution of Bug 1393, disclosing some of the sensitivity in this domain.

NOTICE: Same is true for 'jogamp.opengl.macosx.cgl.MacOSXCGLContext.getUpdateHandle()'
which also acts on the 'useAppKit' flag.

+++

2) Java: CGL.setContextView(long ctx, long view)

2.1) Caller 'jogamp.opengl.macosx.cgl.MacOSXCGLContext.NSOpenGLImpl.associateDrawable(boolean)'

CGL.setContextView(long ctx, long view) only called if 'null==backingLayerHost',
aka older onscreen Java-AWT on MacOS?

Good news this method potentially can be issued deferred on MainThread, non-blocking. This is already done for the 

However, I don't see the use case for onscreen Java-AWT,
so only a NEWT onscreen case might be related.

Would like to see your unit test.

+++

1) Java: CGL.createContext(long shareContext, long view, ..

1.1) Caller: 'jogamp.opengl.GLContextImpl.createContextARB(long, boolean)'

Probably as hard to resolve as 3.1)

+++++++++++++++++++++++

Bottom line, it might be OK to at least see to a non blocking resolution.

Information I need to have first:

A) Unit test exposing this issue.

B) Detailed description of running system exposing this issue.

C) Original application 'suffering' having this issue.

Please note that my systems unit tests all pass on Catalina with latest XCode 11.
I also tested with property 'nativewindow.debug.OSXUtil.MainThreadChecker' set,
i.e. running using MacOS MainThreadChecker.
While some soft issues may still occur, no crash has been experienced.
See Bug 1393 comment 2

But I am sure we will resolve this issue for all parties involved if committed.

Created attachment 838 [details]
Test case

System info:

Model: MacBookPro11,3, BootROM 157.0.0.0.0, 4 processors, Quad-Core Intel Core i7, 2.5 GHz, 16 GB, SMC 2.19f12
Graphics: kHW_IntelIrisProItem, Intel Iris Pro, spdisplays_builtin
Graphics: kHW_NVidiaGeForceGTX750MItem, NVIDIA GeForce GT 750M, spdisplays_pcie_device, 2 GB
Memory Module: BANK 0/DIMM0, 8 GB, DDR3, 1600 MHz, 0x80AD, 0x484D54343147533641465238412D50422020
Memory Module: BANK 1/DIMM0, 8 GB, DDR3, 1600 MHz, 0x80AD, 0x484D54343147533641465238412D50422020
AirPort: spairport_wireless_card_type_airport_extreme (0x14E4, 0x134), Broadcom BCM43xx 1.0 (7.77.105.1 AirPortDriverBrcmNIC-1429)
Bluetooth: Version 7.0.0f8, 3 services, 27 devices, 1 incoming serial ports
Network Service: Wi-Fi, AirPort, en0
Serial ATA Device: APPLE SSD SM0512F, 500.28 GB
USB Device: USB 3.0 Bus
USB Device: Apple Internal Keyboard / Trackpad
USB Device: BRCM20702 Hub
USB Device: Bluetooth USB Host Controller
Thunderbolt Bus: MacBook Pro, Apple Inc., 17.1


Command line:

./make.sh && ./Bug1398macOSContextOpsOnMainThread /Library/Java/JavaVirtualMachines/adoptopenjdk11.jdk/Contents/Home/lib/jli/libjli.dylib


The attachment contains a diff for a portion of the native launcher code from the "suffering" application and a simple Java class to reproduce the issue. There is no need for the JFrame/GLCanvas etc...only thing needed is:

static {
        GLProfile.initSingleton();
}


I also pulled the repos today before testing to make sure I was up-to-date.

(In reply to jani from comment #8)
Excellent, thank for the test case and description jani.

So I assume this test case reliably reproduces the issue
or is it sporadic?

I will test hopefully be able to test this case soon.

We have to see if this is just a corner case of using XCode < 11
or not.

However, I agree with the general sentiment that an overall solution 
for the setView and createContext should be found.
I continue thinking about such and will tackle it from the java side.

Goal is to have these items async on MainThread if anyhow possible,
similar to the calayer invocations.
This guarantees a dead-lock free implementation.

I also understand the setView case for non calayer invocation now,
so all use case question should be resolved by now.

It is repeatable every time. If I build with [ctx setView] it will always crash, if I push it to main thread it always works and I have not seen any deadlock issues. I've had a patched version of JOGL bundled with a product since Nov 2019 and have not heard of issues so far.

Here is my build setup:

Xcode 11.1
Build version 11A1027

openjdk version "11.0.5" 2019-10-15
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.5+10)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.5+10, mixed mode)


Let me know if there is anything else I can do or help with. Maybe I can take a look at how the CALayer calls are handled and see if I can figure how to make my patch async :D

(In reply to jani from comment #10)
> It is repeatable every time. If I build with [ctx setView] it will always
> crash, 

Good that your test case crashes reliably, which eases validation

> if I push it to main thread it always works and I have not seen any
> deadlock issues. 
> I've had a patched version of JOGL bundled with a product since Nov 2019 and 
> have not heard of issues so far.

That is hardly a qualifying statement.

All unit tests passed as-is <https://jogamp.org/chuck/job/jogl/1503/label=macosx-10_6-x86_64-nvidia/testReport/>

Plus validation from a application maintainer testing on MacOS.

As I wrote in comment 7, I already ran into a deadlock with a similar change.

Further I would like to identify the difference which causes the crash
versus our normal working use-case.

+++

> Let me know if there is anything else I can do or help with. Maybe I can take a 
> look at how the CALayer calls are handled and see if I can figure how to make 
> my patch async :D

I will comment on this next week.

Thank you Jani for elaborating on this issue here.

I finally was able to reproduce the issue and figured out
the culprit introducing this crash
within '[NSOpenGLContext setView:]'.

XCode 11 _with_ using its default SDK 'macosx10.15' introduces this crash,
while using SDK 'macosx10.11' does not (even using XCode 11).

Below a list of commits adding this native test.

I will follow up with a fix.

Thank you.

+++

commit 4fad4869d4a929739c830f6ce3ac171d8dd5427a

    Bug 1398: Importing Jani's native test, attempting to reproduce the crash using XCode 11 and JOGL 2.3.2 as well as current tip
    
    This change imports 'jani@nexcus.com' patch as
    reported and provided in Bug 1398.
    
    I can not execute this patch properly,
    as received a error message regarding missing Info.plist at start.


commit f4f92cdc0eb89c62070a865601527097e6d5cc72

    Bug 1398: Fixing native test allowing its execution using JOGL 2.3.2 and latest tip using XCode 11
    
    Changes to test
    - Using own 'NSApplicationMain' entry to avoid the 'missing Info.plist' message!
    
    - Configurable CLASSPATH and LIBPATH at compile time.
      Note that the java.library.path is now hardcoded as well.
    
    - Don't close stderr in test, just fflush
    
    - Don't close the JVM after launch via 'die(env)', let it run.
    
    - Java: Add GLEventListener RedSquareES2 to see something in action @ 30fps
    
    - Java: GLCanvase visible bounds


commit ff780fc11602fb79a7ce1dcf879fdaeb865b9fa8

    Bug 1398: Refine test case: Make classpath and libpath runtime configurable + show JOGL version


commit 78b96b89a68ff35969aea83de294cd3cc1178f26

    Bug 1398: Crash only occurs @ -[NSOpenGLContext setView:] when using XCode 11 _and_ its default SDK 'macosx10.15'
    
    This patch demonstrates that using the SDK 'macosx10.11' does not cause the crash @ -[NSOpenGLContext setView:].
    
    SDK 'macosx10.15' enforces Apple's own Cargo Cult of 'main-thread' by throwing a SIGILL signal (or SIGABRT)
    - essentially an exception.
    
    This surely renders our code officially invalid due to this policy,
    i.e. we are not allowed to issue [* setView] on any non main-thread.
    
    +++
    
    The crash occurs independently of used Java version on Java 8 - 11,
    as well as on JogAmp 2.3.2 - current master tip.
    
    +++
    
    The initial remedy to issue said action on the main-thread in a blocking/wait
    manner has the risk to deadlock, due to
    
    1) [NSOpenGLContext setView:] itself using a mutex (Thanks to Ken Harris's analysis)
    
    and
    
    (2) in case where we are 'thread hopping':
    - [main-thread] Event like 'window ready' -> kick off action on EDT-thread *blocking*
    - [EDT-thread] Create stuff incl OpenGLContext -> kick off setView on main-thread *blocking*
    
    This has to be further investigated.
    
    This crash finally has been reliably reproduced now.

commit d1a4d790c89934616fa1883312b4064bda9fa420

Bug 1398: MacOS: Perform [NSOpenGLContext setView:] on main-thread async w/o blocking
    
Set NSOpenGLContext's NSView via [NSOpenGLContext setView:]
on the main-thread as enforced since XCode 11 using SDL macosx10.15, using Runnable SetNSViewCmd.
    
This operation must be performed async w/o blocking to allow
other tasks locking the NativeSurface on main-thread to complete.
    
Further, since [NSOpenGLContext setView:] acquired the CGLContext lock,
it can't be locked until this task has been completed.
    
Worst case scenario for a late [NSOpenGLContext setView:] issuance
might be corrupt initial frame(s) displayed.
    
Since all concurrent locking is performed within JOGL,
the unlocked CGLContext window risk is only academic.
However, if native 3rd party toolkits take share control,
we might have a situation.
    
+++ 
    
SetNSViewCmd is issued @ makeCurrent() now as opposed to createContext(..)
and associateDrawable(true). The latter was actually late as well,
as it also happened after makeCurrent when updating the drawable
association. It also missed setting a null NSView when detached!
    
release() will also set a null NSView if called after associateDrawable(false).
    
SetNSViewCmd will only be issued if the NSView has been changed,
i.e. first makeCurrent() or changing the drawable.
If issued, makeCurrent() will not lock the underlying CGLContext
and hence allow SetNSViewCmd to perform - see above.
    
+++ 
    
NSViewDescriptor class structure replaces the less convenient method 'getNSViewHandle(..)',
exposing all collected drawable characteristics as fields.
NSViewDescriptor also respects a ProxySurface's OPT_UPSTREAM_SURFACELESS mode,
which results in not using any underlying NSView - similar to OPT_UPSTREAM_WINDOW_INVISIBLE.
    
This change ensures that all surfaceless GL operations will not use any NSView.

854924c72599aab8d8193b9cb2b85a25bd395a2d merge

+++

Following commit 36ca7245653b1a0897f2070b9acbe0f0898f5949 
further tests fix of commit d1a4d790c89934616fa1883312b4064bda9fa420.

It uses SWT tests being invoked not from the main-thread.

They probably mark the most API heavy UI thread 'hopping' 
test cases - hence help validating the fix.

Now analyzing test results.
 
+++

36ca7245653b1a0897f2070b9acbe0f0898f5949

OSX/SWT Testing: Drop using 'com.jogamp.newt.util.MainThread' enforcing default test behavior
    
SWT and OSX's UI TK have their strict threading policy we require to comply with, e.g. see Bug 1398 lately.
    
It doesn't help using our own MainThread vehicle to move the unit test on the OS main thread,
as this removes potential causes of deadlocks - which we intend to find and resolve.
    
This patch removed using MainThread altogether from our ant unit testing recipe
as well from our manual test scripts.
    
Unit tests are no more executed on the 'main thread'.
    
SWT tests are patched to comply with SWT's UI threading policy.
    
We also catch violations within NewtCanvasSWT and our SWT GLCanvas
to provide same behavior on all platforms.

commit 3e141416ea6c85c14dc622dae57f071d5fd0ff4f

    Bug 1398: Expose NativeSurface implementation's RecursiveLock if utilized
    
    This prepares proper release of the acquired NativeSurface lock to cure the missing CGLContext lock, see followup commit.

+++

commit d88ca606f67e16c144b36f8fd1f188fdf8531ee0

    Bug 1398: Ensure CGLContext lock will be acquired before leaving user makeCurrent() call
    
    Command SetNSViewCmd sets NSOpenGLContext's NSView via [NSOpenGLContext setView:]
    on the main-thread as enforced since XCode 11 using SDK macosx10.15, see Bug 1398.
    
    This command is injected into OSX's main-thread @ NSOpenGLImpl.makeCurrent(long) only if required,
    i.e. issued only for a newly bound NSView and skipped for surface-less or offscreen 'surfaces'.
    
    This operation must be performed w/o blocking other tasks locking the NativeSurface on main-thread to complete.
    
    Since [NSOpenGLContext setView:] acquires the CGLContext lock on the main-thread,
    it can't be locked by the calling thread until this task has been completed.
    Command issuer NSOpenGLImpl.makeCurrent(long) will not acquire the CGLContext lock if this command is pending.
    
    contextMadeCurrent(true) cures the potential unlocked CGLContext by issuing
    a whole GLContext.release() and GLContext.makeCurrent() cycle while waiting for this command to be completed in-between.
    This GLContext cycle also ensures an unlocked NativeSurface.getLock() in-between,
    allowing potentially blocked other tasks on the main-thread to complete and hence this queued command to execute.
    
    Notable test provoking critical multithreading issues is com.jogamp.opengl.test.junit.jogl.demos.es2.newt.TestGearsES2NewtCanvasSWT.
    Notable test exposing issues with an unlocked CGLContext is com.jogamp.opengl.test.junit.jogl.glsl.TestGLSLShaderState02NEWT.

+++

commit 0779f229b0e9538c640b18b9a4e095af1f5a35b3 (HEAD -> master, origin/master, origin/HEAD, jordan/master, jogamp/master, jausoft/master, gitlab-jogamp/master, github-jogamp/master)

    Add missing SWTTestUtil, missed in commit 36ca7245653b1a0897f2070b9acbe0f0898f5949

New RC including the fix
<https://jogamp.org/deployment/archive/rc/v2.4.0-rc-20200307/>

Jenkins build
<https://jogamp.org/chuck/view/fwd/job/jogl/1506/>

The following subsequent issue has been identified:

A deadlock happens due to an AWT action on Appkit
flushing the AWT-Event (issuing a Runnable and waiting),
which blocks the Appkit -

while we are sitting on the AWT EDT,
and issuing our SetNSView command on AppKit and waiting,
which conversely blocks the AWT EDT.

Created attachment 839 [details]
Reproduction video of application hang

Created attachment 840 [details]
Sample project to reproduce the hang of an application

Created attachment 841 [details]
Log information when the application is hung by menu operation

Created attachment 842 [details]
Dump information when the application is hung by menu operation

Created attachment 843 [details]
Log information when the application hangs due to window size change

Created attachment 844 [details]
Dump information when the application hangs due to window size change

(In reply to Sven Gothel from comment #17)

Resolved via ...

commit 9e8a24933e9f396406f895ec137d18aefb1c2fe8

    Bug 1398: Avoid AWT-AppKit blocking feedback flush deadlock and SetNSViewCmd on initial makeCurrent when offscreen
    
    makeCurrent shall skip SetNSViewCmd for offscreen, i.e. refine criteria of nsViewChanged.
    Previous term enforced SetNSViewCmd on initial call as lastNSViewDescr was null.
    Expand first term to require an actual non null NSView.
    
    contextMadeCurrent must avoid blocking to wait for completion of our SetNSViewCmd on AppKit.
    AWT has procedures running on AppKit under certain situations,
    where it issues a feedback flush on AWTEDT (from Appkit) blocking.
    This in turn deadlocks our SetNSViewCmd waiting on the AppKit,
    as we are blocking the AWTEDT waiting for same command.
    
    Further avoiding other potential deadlocks, by adding a 500ms timeout.
    Also clearing the lastSetNSViewCmd field post wait, regardless,
    which avoid repeatitive SetNSViewCmd issuance on timeout.
    Note that the SetNSViewCmd, we failed to wait for eventually gets executed.

(In reply to Jun Koyama from comment #23)

yes, same issue as fixed with last commit see comment 24

(In reply to Sven Gothel from comment #25)

Thank you for the fix commit.
I built the library manually and confirmed that the deadlock no longer occurs in the previously attached sample project.

The manual build was done by skipping the test case because it fails due to a symbol error in "TestBug1398Deadlock02AWT.java".

see comment 26 and comment 24