Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Crash due to wl_frame_callback_handler() called after window destruction #24013

Closed
vadz opened this issue Oct 29, 2023 · 2 comments
Closed

Crash due to wl_frame_callback_handler() called after window destruction #24013

vadz opened this issue Oct 29, 2023 · 2 comments
Milestone
3.2.4

Comments

@vadz
Copy link
Contributor

vadz commented Oct 29, 2023

Not sure when exactly was this introduced, but there is an easily reproducible crash in the cube sample:

  1. Launch it.
  2. Start the timer (by pressing Space).
  3. Close the window by using the title bar button.
Valgrind output showing more details 
Invalid read of size 8
   at 0x486F377: wxGLCanvasEGL::OnWLFrameCallback() (glegl.cpp:448)
   by 0x486F604: wl_frame_callback_handler (glegl.cpp:503)
   by 0x740AF79: ??? (in /usr/lib/x86_64-linux-gnu/libffi.so.8.1.2)
   by 0x740A40D: ??? (in /usr/lib/x86_64-linux-gnu/libffi.so.8.1.2)
   by 0x740AB0C: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.8.1.2)
   by 0x6460760: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.21.0)
   by 0x645CAA9: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.21.0)
   by 0x645E41B: wl_display_dispatch_queue_pending (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.21.0)
   by 0x645E96E: wl_display_roundtrip_queue (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.21.0)
   by 0x630850B: gdk_flush (in /usr/lib/x86_64-linux-gnu/libgdk-3.so.0.2406.32)
   by 0x5CB749E: gtk_main (in /usr/lib/x86_64-linux-gnu/libgtk-3.so.0.2406.32)
   by 0x4E597E6: wxGUIEventLoop::DoRun() (evtloop.cpp:61)
 Address 0xe1057b8 is 488 bytes inside a block of size 912 free'd
   at 0x484399B: operator delete(void*, unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x12416A: TestGLCanvas::~TestGLCanvas() (cube.h:64)
   by 0x50A1ECE: wxWindowBase::Destroy() (wincmn.cpp:570)
   by 0x50A2017: wxWindowBase::DestroyChildren() (wincmn.cpp:602)
   by 0x4E7FA8F: wxWindow::~wxWindow() (window.cpp:2900)
   by 0x4E630C3: wxNonOwnedWindowBase::~wxNonOwnedWindowBase() (nonownedwnd.h:24)
   by 0x4E62C3A: wxNonOwnedWindow::~wxNonOwnedWindow() (nonownedwnd.cpp:218)
   by 0x5096257: wxTopLevelWindowBase::~wxTopLevelWindowBase() (toplvcmn.cpp:99)
   by 0x4E716E3: wxTopLevelWindowGTK::~wxTopLevelWindowGTK() (toplevel.cpp:930)
   by 0x4E22FD9: wxTopLevelWindow::~wxTopLevelWindow() (toplevel.h:395)
   by 0x4FE074F: wxFrameBase::~wxFrameBase() (framecmn.cpp:167)
   by 0x11F88B: wxFrame::~wxFrame() (frame.h:16)
 Block was alloc'd at
   at 0x4840F2F: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x11B7B3: MyFrame::MyFrame(bool) (cube.cpp:459)
   by 0x11AAE4: MyApp::OnInit() (cube.cpp:260)
   by 0x11E2F4: wxAppConsoleBase::CallOnInit() (app.h:92)
   by 0x54B3E24: wxEntry(int&, wchar_t**) (init.cpp:525)
   by 0x54B3F25: wxEntry(int&, char**) (init.cpp:552)
   by 0x11AA1A: main (cube.cpp:253)

I'm going to fix this by using weak references to the canvas.
@vadz vadz self-assigned this Oct 29, 2023
@joanbm
Copy link
Contributor

joanbm commented Oct 29, 2023 •
edited

Thanks for reporting, I also found this issue recently as well.

The problem was introduced by #23835 (included in wxWidgets 3.2.3) and it happens because I wrongly assumed that before the GTK widget's unmap signal would be triggered before the wxGLCanvas destructor, but that does not appear to necessarily happen. This causes some resources to be leaked including the Wayland frame callback. When the callback runs, the wxGLCanvas has already been destroyed, so it's a use-after-free, and causes a crash sometimes.

To fix it it should be enough to add a call to wxGLCanvas::DestroyWaylandSubsurface from ~wxGLCanvas::wxGLCanvas. I'll submit a PR soon.

@vadz vadz removed their assignment Oct 29, 2023
@vadz
Copy link
Contributor Author

vadz commented Oct 29, 2023

Ah, I see, thanks. This is indeed even better (and much easier to backport to 3.2) than using a weak reference, TIA!

@vadz vadz added this to the 3.2.4 milestone Oct 29, 2023
joanbm added a commit to joanbm/wxWidgets that referenced this issue Oct 29, 2023
@joanbm
Clean up Wayland resources on wxGLCanvas destructor
24c690a 
If the wxGLCanvas is destroyed immediately (without hiding it first),
the GTKs widget's `unmap` signal which usually destroys the Wayland
resources is not emitted. Thus, we need to ensure they are destroyed
on the destructor instead.

This fixes an use-after-free issue, sometimes causing a crash, because
one of the leaked resources is the canvas's Wayland frame callback.

Fixes wxWidgets#24013
@joanbm joanbm mentioned this issue Oct 29, 2023
Closed
@vadz vadz closed this as completed in 22ae7a5 Oct 29, 2023
vadz pushed a commit to vadz/wxWidgets that referenced this issue Oct 29, 2023
@joanbm @vadz
Clean up Wayland resources on wxGLCanvas destructor
3d235b8 
If the wxGLCanvas is destroyed immediately (without hiding it first),
the GTKs widget's `unmap` signal which usually destroys the Wayland
resources is not emitted. Thus, we need to ensure they are destroyed
on the destructor instead.

This fixes an use-after-free issue, sometimes causing a crash, because
one of the leaked resources is the canvas's Wayland frame callback.

See wxWidgets#24013, wxWidgets#24016.

(cherry picked from commit 22ae7a5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.2.4
Development

Successfully merging a pull request may close this issue.

Clean up Wayland resources on wxGLCanvas destructor joanbm/wxWidgets
2 participants
@vadz @joanbm