We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 3996 - Corrupted or over-released critical section on SDL_Quit (SDL_event_watchers_lock)
Summary: Corrupted or over-released critical section on SDL_Quit (SDL_event_watchers_l...
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: events (show other bugs)
Version: 2.0.7
Hardware: x86_64 Windows 10
: P2 minor
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-12-09 01:04 UTC by Andrew
Modified: 2017-12-13 00:11 UTC (History)
2 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew 2017-12-09 01:04:22 UTC 
I created a simple SDL program that just creates a window, polls an event loop, and bails out and calls SDL_DestroyWindow/SDL_Quit on window close.

Then i ran it underneath appverifier on win10 x64.

SDL appears to be leaking critical sections:

=======================================
VERIFIER STOP 0000000000000209: pid 0x4284: Critical section over-released or corrupted. 

	00007FF7D78B0420 : Critical section address. Run !cs -s <address> to get more information.
	FFFFFFFFFFFFFFFF : Lock count.
	0000000000000000 : Expected lock count.
	0000023891AE0FD0 : Critical section debug info address.


=======================================
This verifier stop is continuable.
After debugging it use `go' to continue.

=======================================

Project1.exe has triggered a breakpoint.

Stack trace for the failure (note, line numbers are off by one, thanks to some fun debugger behavior, the function names are correct))
 	vrfcore.dll!VerifierStopMessageEx()	
 	vfbasics.dll!VerifierStopMessage()	
 	vfbasics.dll!AVrfpVerifyCriticalSectionOwner()	
 	vfbasics.dll!AVrfpRtlLeaveCriticalSection()	
 	SDL2.dll!SDL_UnlockMutex_REAL(SDL_mutex * mutex) Line 105	
>	SDL2.dll!SDL_StopEventLoop() Line 373	
 	SDL2.dll!SDL_QuitSubSystem_REAL(unsigned int flags) Line 315	
 	SDL2.dll!SDL_VideoQuit_REAL() Line 2740	
 	SDL2.dll!SDL_QuitSubSystem_REAL(unsigned int flags) Line 296	
 	SDL2.dll!SDL_Quit_REAL() Line 357	
 	SDL2.dll!SDL_Quit() Line 89	
 	Project1.exe!wWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpCmdLine, int nCmdShow) Line 36	


Since this is SDL_Quit this probably isn't too critical, but it might indicate some kind of corruption happened much earlier that could cause problems over time.
Comment 1 Andrew 2017-12-09 01:05:14 UTC 
(sorry, not leaking, mis-handling)
Comment 2 Andrew 2017-12-09 01:15:51 UTC 

This likely comes down to an additional 'unlock' being called before destroying it, without a matching 'lock'.
Comment 3 Daniel Knobe 2017-12-12 21:50:20 UTC 
An WinRT application crashes when std::recursive_mutex unlock is called without lock is called which is always the case on SDL_Quit() in single threaded application.
Comment 4 Andrew 2017-12-12 22:23:24 UTC 
(In reply to Daniel Knobe from comment #3)
> An WinRT application crashes when std::recursive_mutex unlock is called
> without lock is called which is always the case on SDL_Quit() in single
> threaded application.

Yeah, WinRT being more restrictive than standard win32 doesn't surprise me. SDL_Quit shouldn't need to call unlock prior to destroying it, it should be ensuring that any threads internal to SDL have already been told to shut down and cleaned up prior to deleting the critical section. I can't think of a platform where it's safe to unlock a mutex that doesn't have any valid locks.
Comment 5 Sam Lantinga 2017-12-13 00:11:31 UTC 
Fixed, thanks!
https://hg.libsdl.org/SDL/rev/f0aad4dbf696