Crash on exit "Error calling SDL_DestroyWindow: Window data not set"

RhetTbull commented 1 year ago

Summary

On exit of any ppb example, python crashes with error: "Error calling SDL_DestroyWindow: Window data not set". Traceback below. The examples all appear to run correctly until the the window is closed at which time it looks like Renderer.__exit__ triggers the exception when attempting to call sdl_call(SDL_DestroyWindow, self.window). If pysdl2-dll is downgraded to the last version, the exception is not raised.

Configuration:

platform=MacOS 10.15.7 python=3.10.5 ppb=1.1 pysdl2=0.9.14 pysdl2-dll=2.24.0

All code is running in a dedicated venv and was installed with pip install ppb

Example code to trigger the problem:

Problem is triggered on exit of any ppb example in pursuedpybear/examples. The examples appear to run correctly but when the window is closed, the exception is raised and traceback triggered.

The following is the minimum code required to trigger the exception:

import ppb

def setup(scene):
    pass

ppb.run(setup)

On running, this opens a blank blue window. Closing the window triggers the traceback.

Other relevant details:

On the suggestion of @AstraLuma on Discord, I downgraded pysdl2-dll to the last release (2.0.22.post1) and the examples appear to run correctly without triggering the exception. I will continue to use pysdl2-dll==2.0.22.post1 for now unless I run into other issues with it.

As an aside, what appears to be an unnecessary warning is printed to stderr when running the code:

$ python bug.py
UserWarning: Using SDL2 binaries from pysdl2-dll 2.0.22.post1

Traceback

UserWarning: Using SDL2 binaries from pysdl2-dll 2.24.0
Traceback (most recent call last):
  File "/Users/rhet/Dropbox/Code/ppb/bug.py", line 6, in <module>
    ppb.run(setup)
  File "/Users/rhet/.pyenv/versions/3.10.5/envs/ppb-3-105/lib/python3.10/site-packages/ppb/__init__.py", line 152, in run
    with make_engine(setup, starting_scene=starting_scene, title=title, **engine_opts) as eng:
  File "/Users/rhet/.pyenv/versions/3.10.5/envs/ppb-3-105/lib/python3.10/site-packages/ppb/engine.py", line 299, in __exit__
    self.children.__exit__(*exc)
  File "/Users/rhet/.pyenv/versions/3.10.5/envs/ppb-3-105/lib/python3.10/site-packages/ppb/engine.py", line 220, in __exit__
    self._stack.close()
  File "/Users/rhet/.pyenv/versions/3.10.5/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/contextlib.py", line 584, in close
    self.__exit__(None, None, None)
  File "/Users/rhet/.pyenv/versions/3.10.5/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/contextlib.py", line 576, in __exit__
    raise exc_details[1]
  File "/Users/rhet/.pyenv/versions/3.10.5/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/contextlib.py", line 561, in __exit__
    if cb(*exc_details):
  File "/Users/rhet/.pyenv/versions/3.10.5/envs/ppb-3-105/lib/python3.10/site-packages/ppb/systems/renderer.py", line 170, in __exit__
    sdl_call(SDL_DestroyWindow, self.window)
  File "/Users/rhet/.pyenv/versions/3.10.5/envs/ppb-3-105/lib/python3.10/site-packages/ppb/systems/sdl_utils.py", line 54, in sdl_call
    raise SdlError(f"Error calling {func.__name__}: {err.decode('utf-8')}")
ppb.systems.sdl_utils.SdlError: Error calling SDL_DestroyWindow: Window data not set

RhetTbull commented 1 year ago

Just to confirm it wasn't an issue with my python version (which was built from source using pyenv), I tried with the latest 3.10.7 version from python.org but still got the same error.

$ python --version
Python 3.10.7
$ python bug.py
UserWarning: Using SDL2 binaries from pysdl2-dll 2.24.0
Traceback (most recent call last):
  File "/Users/rhet/Dropbox/Code/ppb/bug.py", line 6, in <module>
    ppb.run(setup)
  File "/Users/rhet/.pyenv/versions/ppb-3-10-7/lib/python3.10/site-packages/ppb/__init__.py", line 152, in run
    with make_engine(setup, starting_scene=starting_scene, title=title, **engine_opts) as eng:
  File "/Users/rhet/.pyenv/versions/ppb-3-10-7/lib/python3.10/site-packages/ppb/engine.py", line 299, in __exit__
    self.children.__exit__(*exc)
  File "/Users/rhet/.pyenv/versions/ppb-3-10-7/lib/python3.10/site-packages/ppb/engine.py", line 220, in __exit__
    self._stack.close()
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/contextlib.py", line 584, in close
    self.__exit__(None, None, None)
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/contextlib.py", line 576, in __exit__
    raise exc_details[1]
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/contextlib.py", line 561, in __exit__
    if cb(*exc_details):
  File "/Users/rhet/.pyenv/versions/ppb-3-10-7/lib/python3.10/site-packages/ppb/systems/renderer.py", line 170, in __exit__
    sdl_call(SDL_DestroyWindow, self.window)
  File "/Users/rhet/.pyenv/versions/ppb-3-10-7/lib/python3.10/site-packages/ppb/systems/sdl_utils.py", line 54, in sdl_call
    raise SdlError(f"Error calling {func.__name__}: {err.decode('utf-8')}")
ppb.systems.sdl_utils.SdlError: Error calling SDL_DestroyWindow: Window data not set

RhetTbull commented 1 year ago

The error appears to be coming from SDL here:

https://github.com/libsdl-org/SDL/blob/00452e47fa022314b4c21af2d4e5b63a53f89b8f/src/video/cocoa/SDL_cocoawindow.m#L2242-L2252

int
Cocoa_GetWindowDisplayIndex(_THIS, SDL_Window * window)
{ @autoreleasepool
{
    NSScreen *screen;
    SDL_WindowData *data = (__bridge SDL_WindowData *) window->driverdata;

    /* Not recognized via CHECK_WINDOW_MAGIC */
    if (data == nil) {
        return SDL_SetError("Window data not set");
    }

The code in question was changed recently.

Prior to the change, the code return 0 if window data == nil:

    /* Not recognized via CHECK_WINDOW_MAGIC */
    if (data == nil) {
        return 0;
    }

a-hurst commented 1 year ago

@RhetTbull Just chiming in here as the maintainer for PySDL2: the underlying issue here I think is that the SDL2 people have started to make heavier use of SDL_SetError for non-fatal errors, but I believe PPB's error handling is designed to always check if SDL_GetError is non-empty after calling a given function and raises an error if one was found.

I had to rewrite a bunch of PySDL2's unit test suite to work around this problem for the latest release, so it makes sense it would also cause problems for PPB. The official guidance I've gotten from the SDL2 devs is to only check for errors if an error code or null pointer was received from the function.

ppb / pursuedpybear