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

Fix the heap-use-after-free and memory leak #54

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

weihsinyeh
Copy link
Collaborator

To fix the heap-use-after-free issue when screen is accessed by the twin_sdl_work() function, remove the _twin_sdl_destroy(screen, tx) operation. Additionally, to address the memory leak, add the twin_path_destroy(path) function to ensure the unused path is properly destroyed.

Close #49

@weihsinyeh
Copy link
Collaborator Author

However, if this is the solution, the _twin_sdl_destroy() function will no longer be used.

@jserv
Copy link
Contributor

jserv commented Oct 1, 2024

However, if this is the solution, the _twin_sdl_destroy() function will no longer be used.

Quote from SDL manual about SDL_Quit

You should call this function even if you have already shutdown each initialized subsystem with SDL_QuitSubSystem(). It is safe to call this function even in the case of errors in initialization.
... You can use this function with atexit() to ensure that it is run when your application is shutdown, but it is not wise to do this from a library or other dynamically loaded code.

Since checking for events is no longer allowed after invoking SDL_Quit(), we need to find a way to properly deallocate SDL-specific resources.

@weihsinyeh
Copy link
Collaborator Author

AddressSanitizer reports:

==12685==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 17 byte(s) in 1 object(s) allocated from:
    #0 0x55ddca5715b8 in __interceptor_malloc (/home/weihsin/linux2024/tmp/mado/demo-sdl+0xf95b8)
    #1 0x7fd3bf6a5896 in _XlcDefaultMapModifiers (/lib/x86_64-linux-gnu/libX11.so.6+0x5b896)

SUMMARY: AddressSanitizer: 17 byte(s) leaked in 1 allocation(s).

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Avoid using backticks in git commit messages and comments to ensure compatibility with terminal emulators that may not render the backtick character properly.

Instead, use the pair ' or " (quotation marks).

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Append Close #49 at the end of git commit message since this pull request does resolve the issue.

To fix the heap-use-after-free issue when 'screen' is accessed by the
'twin_sdl_work()' function, remove the '_twin_sdl_destroy(screen, tx)'
operation. Additionally, to address the memory leak, add the
'twin_path_destroy(path)' function to ensure the unused path is properly
destroyed. Add the operation to deallocate 'frame', as it will no longer
be used in the future.

Close sysprog21#49
@jserv jserv merged commit 049943e into sysprog21:main Oct 2, 2024
3 checks passed
@jserv
Copy link
Contributor

jserv commented Oct 2, 2024

Thank @weihsinyeh for contributing!

@weihsinyeh weihsinyeh deleted the memoryleak branch October 2, 2024 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory bugs reported by AddressSanitizer
2 participants