generic-library/breakpad
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Don't do work inside assert(). Ever.

Browse Source 
The Mac crash key manipulation code was intended to be thread-safe through the
provision of a mutex. The mutex operations were done inside an assert().
assert() is a no-op in NDEBUG (release) builds. Therefore, in release builds,
these operations were occurring without being protected by any mutex at all,
and were nowhere near thread-safe.

BUG=chromium:331268
R=rsesek@chromium.org

Review URL: https://breakpad.appspot.com/1034002

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1270 4c0a9323-5329-0410-9bdc-e9ce6186880e
This commit is contained in:
mark@chromium.org 2014-01-10 19:54:20 +00:00
parent 1e99113fbe
commit 3ea04ec479
2 changed files with 26 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
src/client/ios/Breakpad.mm
View File

@ -31,6 +31,7 @@
#import "client/ios/Breakpad.h"
#include <assert.h>
#import <Foundation/Foundation.h>
#include <pthread.h>
#include <sys/stat.h>
@ -91,36 +92,32 @@ pthread_mutex_t gDictionaryMutex;
// ProtectedMemoryLocker will unprotect this block after taking the lock.
// Its destructor will first re-protect the memory then release the lock.
class ProtectedMemoryLocker {
public:
// allocator may be NULL, in which case no Protect() or Unprotect() calls
// will be made, but a lock will still be taken
public:
ProtectedMemoryLocker(pthread_mutex_t *mutex,
ProtectedMemoryAllocator *allocator)
: mutex_(mutex), allocator_(allocator) {
: mutex_(mutex),
allocator_(allocator) {
// Lock the mutex
assert(pthread_mutex_lock(mutex_) == 0);
int rv = pthread_mutex_lock(mutex_);
assert(rv == 0);
// Unprotect the memory
if (allocator_ ) {
allocator_->Unprotect();
}
allocator_->Unprotect();
}
~ProtectedMemoryLocker() {
// First protect the memory
if (allocator_) {
allocator_->Protect();
}
allocator_->Protect();
// Then unlock the mutex
assert(pthread_mutex_unlock(mutex_) == 0);
int rv = pthread_mutex_unlock(mutex_);
assert(rv == 0);
};
private:
// Keep anybody from ever creating one of these things not on the stack.
ProtectedMemoryLocker() { }
private:
ProtectedMemoryLocker();
ProtectedMemoryLocker(const ProtectedMemoryLocker&);
ProtectedMemoryLocker & operator=(ProtectedMemoryLocker&);
ProtectedMemoryLocker& operator=(const ProtectedMemoryLocker&);
pthread_mutex_t *mutex_;
ProtectedMemoryAllocator *allocator_;

29
src/client/mac/Framework/Breakpad.mm
View File

@ -33,6 +33,7 @@
#import "client/mac/Framework/Breakpad.h"
#include <assert.h>
#import <Foundation/Foundation.h>
#include <pthread.h>
#include <sys/stat.h>
@ -95,36 +96,32 @@ pthread_mutex_t gDictionaryMutex;
// ProtectedMemoryLocker will unprotect this block after taking the lock.
// Its destructor will first re-protect the memory then release the lock.
class ProtectedMemoryLocker {
public:
// allocator may be NULL, in which case no Protect() or Unprotect() calls
// will be made, but a lock will still be taken
public:
ProtectedMemoryLocker(pthread_mutex_t *mutex,
ProtectedMemoryAllocator *allocator)
: mutex_(mutex), allocator_(allocator) {
: mutex_(mutex),
allocator_(allocator) {
// Lock the mutex
assert(pthread_mutex_lock(mutex_) == 0);
int rv = pthread_mutex_lock(mutex_);
assert(rv == 0);
// Unprotect the memory
if (allocator_ ) {
allocator_->Unprotect();
}
allocator_->Unprotect();
}
~ProtectedMemoryLocker() {
// First protect the memory
if (allocator_) {
allocator_->Protect();
}
allocator_->Protect();
// Then unlock the mutex
assert(pthread_mutex_unlock(mutex_) == 0);
int rv = pthread_mutex_unlock(mutex_);
assert(rv == 0);
};
private:
// Keep anybody from ever creating one of these things not on the stack.
ProtectedMemoryLocker() { }
private:
ProtectedMemoryLocker();
ProtectedMemoryLocker(const ProtectedMemoryLocker&);
ProtectedMemoryLocker & operator=(ProtectedMemoryLocker&);
ProtectedMemoryLocker& operator=(const ProtectedMemoryLocker&);
pthread_mutex_t *mutex_;
ProtectedMemoryAllocator *allocator_;