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.

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

Memory leak while setting Desktop display mode (Windows) #7982

Closed
RPP-dev opened this issue Jul 14, 2023 · 2 comments
Closed

Memory leak while setting Desktop display mode (Windows) #7982

RPP-dev opened this issue Jul 14, 2023 · 2 comments
Labels
waiting Waiting on user response
Milestone

Comments

@RPP-dev
Copy link
Contributor

RPP-dev commented Jul 14, 2023

In Windows 10, WIN_AddDisplays() seems to be called twice.

Additonally, in WIN_GetDisplayMode() there is a memory allocation:

data = (SDL_DisplayModeData *)SDL_malloc(sizeof(*data));

The second WIN_AddDisplays() call cause to override the first allocation causing the leak (220 bytes per each monitor you have and each time you call to init and quit the video system).

I dont know why this function is called twice (it could be on purpose or another bug that should be fixed) so, I have a patch that only avoid this memory leak modifying the SDL_SetDesktopDisplayMode() function.

WIN_AddDisplay() addressed a similar memory leak calling to SDL_ResetFullscreenDisplayModes() but nothing is done with the desktop mode.

It could not be the cleanest way of removing the memory leak but it was I am using on my project.

I have a patch here: main...RPP-dev:SDL:main

Before creating the pull request I would like to know your opinion on this fix.

Thanks
Regards

@slouken
Copy link
Collaborator

slouken commented Nov 8, 2023

I'd like to find out why SDL_SetDesktopDisplayMode() is getting called twice. How many monitors do you have connected? Can you print out the value of info->szDevice before it gets called?

@slouken slouken added this to the 2.30.0 milestone Nov 8, 2023
@slouken slouken added the waiting Waiting on user response label Nov 8, 2023
@slouken slouken closed this as completed in bea34c5 Nov 8, 2023
slouken added a commit that referenced this issue Nov 14, 2023
Windows updates the desktop display mode once at video init (in WIN_InitModes()) and once when creating a window (in WIN_RefreshDisplays())

Fixes #7982
Fixes #8189

(cherry picked from commit bea34c5)
slouken added a commit that referenced this issue Nov 14, 2023
Windows updates the desktop display mode once at video init (in WIN_InitModes()) and once when creating a window (in WIN_RefreshDisplays())

Fixes #7982
Fixes #8189

(cherry picked from commit bea34c5)
(cherry picked from commit 0ec1209)
@RPP-dev
Copy link
Contributor Author

RPP-dev commented Feb 17, 2024

Hi, although it is some months late... I have 2 monitors connected.
I can see the fix has been implemented so thank you very much!

No more memory leaks because of this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting Waiting on user response
Projects
None yet
Development

No branches or pull requests

2 participants