Bug 1468 - SIGSEGV on use after free when destroying NEWT Window/Display via a native dispatch'ed event like key/mouse/touch input
Summary: SIGSEGV on use after free when destroying NEWT Window/Display via a native di...
Status: RESOLVED FIXED
Alias: None
Product: Newt
Classification: JogAmp
Component: core (show other bugs)
Version: 2.6.0
Hardware: All all
: P2 major
Assignee: Sven Gothel
URL:
Depends on:
Blocks:
 
Reported: 2023-10-01 19:53 CEST by Sven Gothel
Modified: 2023-10-02 19:43 CEST (History)
0 users

See Also:
Type: DEFECT
SCM Refs:
f842843df2c77f5badaace6858d3336151ce0827
Workaround: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sven Gothel 2023-10-01 19:53:20 CEST
SIGSEGV on use after free of native X11 Display* at XEventsQueued in DisplayDriver.DispatchMessages0.

This potentially happens when an application destroys 
the NEWT Window/Display from an action being called directly
from DisplayDriver.DispatchMessages0 (itself), i.e. keyboard or mouse input.

DisplayDriver.DispatchMessages0 stays in the event loop and the next
XEventsQueued call causes a SIGSEGV due to already deleted 
display driver connection and hence invalid native X11 Display*.

Perhaps mitigate by asking the JavaObject DisplayDriver whether 
it is still alive - since we might have not other means to validate 
the X11 DisplayDriver* after free.
Comment 1 Sven Gothel 2023-10-02 13:09:02 CEST
This issue may also exist for other Windowing System drivers,
where the native (dispatch) method sticks to a loop
and still (re)uses the window or display handle.

One is WindowsWindow, where touch events are looped,
but such handler could have closed the window.
Comment 2 Sven Gothel 2023-10-02 13:55:50 CEST
Querying the status of a window / display instance before dispatching
might not be good enough
- resource could already be GC'ed, so we also would need to query jobject status
- would imply an addition Java callback

+++

Instead, let's do the sweaty work of having the Java callbacks return
a boolean with the value Window.isNativeValid().

This way the dispatch logic
- can bail out right away w/o using the resource anymore

- must be reviewed by myself due to changed Call{Void->Boolean}*(..) 
  invocation change.
  This review shall resolve potential similar issues.

...
Comment 3 Sven Gothel 2023-10-02 19:43:40 CEST
commit f842843df2c77f5badaace6858d3336151ce0827

...

This fix: Having the Java callbacks return
a boolean with the value Window.isNativeValid().
    
This way the dispatch logic
- can bail out right away w/o using the resource anymore
    
- must be reviewed by myself due to changed Call{Void->Boolean}*(..)
  invocation change.
  This review shall resolve potential similar issues.
    
+++ 
    
Tested on X11/Linux/GNU, Windows and MacOS
with new TestDestroyGLAutoDrawableNewtAWT,
which tests all destruction invocation variants.