Memory leak in SDL

DOSBox makes use of the SDL library with a few slight modifications for performance on Windows. This exposes a memory leak in SDL 1.2.x. I have filed a bug report and fix with the SDL people, but since this code path is not used under normal circumstances, only in DOSBox, they are not likely to incorporate it any time soon.

At the suggestion of DOSBox dev c2woody (aka wd) I checked into this. When a program calls SDL_SetVideoMode, the DirectDraw backend in /src/video/windx5/SDL_dx5video.c, function DX5_SetVideoMode allocates a block of memory the size of the frame buffer. It stores this in the pixels member of the surface structure. Unfortunately, the function DX5_LockHWSurface assigns a pointer to the DirectDraw surface to this same member without freeing the original memory block first. That is a memory leak. DOSBox sometimes calls SDL_SetVideoMode in rapid succession, depending on what the programs running inside it do. The Magic Circle is an intro that can leak hundreds of megabytes when run in DOSBox, especially in the final section.

My fix adds an extra member variable to an internal structure that is not exposed to programs using the SDL api and stores the pointer to the buffer in there on the first call to DX5_LockHWSurface. When closing the video subsystem or switching to another video mode, this pointer is then probably released. I could have chosen not to allocate the memory in the first place or release the buffer the moment the pointer was overwritten, but this is more compatible. Although it is bad programming practice, it is actually possible that a program makes a copy of the pointer before it gets overwritten and uses it for its own purposes. Such a program would crash if the observable behaviour changed. In the fix, the observable behaviour is retained at least until the next call to SDL_SetVideoMode.

As this fix is not likely to make it into mainstream SDL any time soon, I'm posting the patch and an updated DLL here:

  1. diff file
  2. pre-built DLL

Comments

No comments, yet...

Post a comment