Von: William A Rowe Jr [mailto:wr...@rowe-clan.net] Gesendet: Dienstag, 19. März 2019 16:36 An: Michael Schlenker <michael.schlen...@contact-software.com> Cc: Stefan Sperling <s...@stsp.name>; dev@apr.apache.org Betreff: Re: APR thread_mutex_cleanup on windows mishandles being called twice > > > > You're right that acting on a corrupt memory is bad. > > > > It looks like your proposed patch detects and then skips a redundant call to > > DeleteCriticalSection() when a mutex is unlocked twice: > > > > if (lock->type == thread_mutex_critical_section) { > > lock->type = -1; > > DeleteCriticalSection(&lock->section); > > + } > > + else if (lock->type == -1) { > > + /* do nothing */ > > } > > > > I'd prefer thread_mutex_cleanup() to loudly complain when this happens, > > and perhaps even abort the program, forcing API users to fix their code. > > Agreed. That should probably be a kind of assert that aborts or fails loud, > not sure what the APR convention/function for that case would be. > > Just initializing the handle member to default INVALID_HANDLE is enough to > prevent the memory corruption/freed random handles.
If this is the consensus, to avoid further corruption, I'd suggest we reset (*mutex)->handle = NULL on first close. Still should trigger a fault where it is double-closed. Won't silently continue to close random handles. And avoids the extra test. Do we agree on that band-aid? The proper reset value wouldn't be NULL but INVALID_HANDLE_VALUE, which is defined like: #define INVALID_HANDLE_VALUE ((HANDLE)(LONG_PTR)-1) Otherwise agreed. Michael
