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

Fix segmentation fault when destroying window #2530

Merged
merged 4 commits into from
Oct 31, 2023
Merged

Fix segmentation fault when destroying window #2530

merged 4 commits into from
Oct 31, 2023

Conversation

novialriptide
Copy link
Member

@novialriptide novialriptide commented Oct 26, 2023 •
edited
Loading

Fixes: #2523

@novialriptide novialriptide requested a review from a team as a code owner October 26, 2023 02:48
@novialriptide novialriptide linked an issue Oct 26, 2023 that may be closed by this pull request
Segfault when draw on display surface after display window is destroyed #2523
Closed
@novialriptide novialriptide marked this pull request as draft October 26, 2023 02:50
ankith26
ankith26 reviewed Oct 28, 2023
View reviewed changes
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure this is the way to proceed on fixing this issue. In general, we allow surface operations even when there is no display surface active. Adding these checks might break existing working code

@novialriptide novialriptide changed the title Check if window is active Fix segmentation fault when destroying window Oct 30, 2023
@novialriptide
Copy link
Member Author

@ankith26 Yeah... I think it's been improved now

@novialriptide novialriptide marked this pull request as ready for review October 30, 2023 01:45
MyreMylar
MyreMylar approved these changes Oct 30, 2023
View reviewed changes
Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Fixes the issue in my testing. You now get a:

Traceback (most recent call last):
  File "C:\Users\dan\Programming\scrap\window_destroy_segfault.py", line 15, in <module>
    sf.fill((0, 0, 0))
pygame.error: Surface is not initialized

When trying any surface operations on the display surface after destroying the window, which seems fair enough to me.

@pmp-p pmp-p merged commit dde0d31 into pygame-community:main Oct 31, 2023
29 checks passed
@ankith26 ankith26 added this to the 2.4.0 milestone Oct 31, 2023
@ankith26 ankith26 added _sdl2 pygame._sdl2 window pygame.Window labels Oct 31, 2023
@novialriptide novialriptide deleted the segfault-fix branch October 31, 2023 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmp-p pmp-p pmp-p approved these changes

@MyreMylar MyreMylar MyreMylar approved these changes

@ankith26 ankith26 Awaiting requested review from ankith26

Assignees
No one assigned
Labels
_sdl2 pygame._sdl2 window pygame.Window
Projects
None yet
Milestone
2.4.0
Development

Successfully merging this pull request may close these issues.

Segfault when draw on display surface after display window is destroyed
4 participants
@novialriptide @MyreMylar @pmp-p @ankith26