From a712aab616b13a9887e00b5d37714f02d7fde6a0 Mon Sep 17 00:00:00 2001 From: Richard Linden <none@none> Date: Fri, 10 Jan 2014 13:56:35 -0800 Subject: [PATCH] added some defensive asserts in lltrace to make cases of misuse more obvious when it crashes --- indra/llcommon/lltracerecording.cpp | 12 +++++++++- indra/llcommon/lltracethreadrecorder.cpp | 30 ++++++++++++------------ 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/indra/llcommon/lltracerecording.cpp b/indra/llcommon/lltracerecording.cpp index cd837c08678..d6232d771d1 100644 --- a/indra/llcommon/lltracerecording.cpp +++ b/indra/llcommon/lltracerecording.cpp @@ -88,6 +88,9 @@ Recording::~Recording() disclaim_alloc(gTraceMemStat, this); disclaim_alloc(gTraceMemStat, mBuffers); + // allow recording destruction without thread recorder running, + // otherwise thread shutdown could crash if a recording outlives the thread recorder + // besides, recording construction and destruction is fine without a recorder...just don't attempt to start one if (isStarted() && LLTrace::get_thread_recorder().notNull()) { LLTrace::get_thread_recorder()->deactivate(mBuffers.write()); @@ -101,7 +104,10 @@ void Recording::update() { mElapsedSeconds += mSamplingTimer.getElapsedTimeF64(); - llassert(mActiveBuffers); + // must have + llassert(mActiveBuffers != NULL + && LLTrace::get_thread_recorder().notNull()); + if(!mActiveBuffers->isCurrent()) { AccumulatorBufferGroup* buffers = mBuffers.write(); @@ -125,12 +131,16 @@ void Recording::handleStart() { mSamplingTimer.reset(); mBuffers.setStayUnique(true); + // must have thread recorder running on this thread + llassert(LLTrace::get_thread_recorder().notNull()); mActiveBuffers = LLTrace::get_thread_recorder()->activate(mBuffers.write()); } void Recording::handleStop() { mElapsedSeconds += mSamplingTimer.getElapsedTimeF64(); + // must have thread recorder running on this thread + llassert(LLTrace::get_thread_recorder().notNull()); LLTrace::get_thread_recorder()->deactivate(mBuffers.write()); mActiveBuffers = NULL; mBuffers.setStayUnique(false); diff --git a/indra/llcommon/lltracethreadrecorder.cpp b/indra/llcommon/lltracethreadrecorder.cpp index a14a8ff035f..187d8546d38 100644 --- a/indra/llcommon/lltracethreadrecorder.cpp +++ b/indra/llcommon/lltracethreadrecorder.cpp @@ -191,25 +191,25 @@ ThreadRecorder::active_recording_list_t::iterator ThreadRecorder::bringUpToDate( void ThreadRecorder::deactivate( AccumulatorBufferGroup* recording ) { active_recording_list_t::iterator recording_it = bringUpToDate(recording); - if (recording_it != mActiveRecordings.end()) + // this method should only be called on a thread where the recorder is active + llassert_always(recording_it != mActiveRecordings.end()); + + ActiveRecording* recording_to_remove = *recording_it; + bool was_current = recording_to_remove->mPartialRecording.isCurrent(); + llassert(recording_to_remove->mTargetRecording == recording); + mActiveRecordings.erase(recording_it); + if (was_current) { - ActiveRecording* recording_to_remove = *recording_it; - bool was_current = recording_to_remove->mPartialRecording.isCurrent(); - llassert(recording_to_remove->mTargetRecording == recording); - mActiveRecordings.erase(recording_it); - if (was_current) + if (mActiveRecordings.empty()) { - if (mActiveRecordings.empty()) - { - AccumulatorBufferGroup::clearCurrent(); - } - else - { - mActiveRecordings.back()->mPartialRecording.makeCurrent(); - } + AccumulatorBufferGroup::clearCurrent(); + } + else + { + mActiveRecordings.back()->mPartialRecording.makeCurrent(); } - delete recording_to_remove; } + delete recording_to_remove; } ThreadRecorder::ActiveRecording::ActiveRecording( AccumulatorBufferGroup* target ) -- GitLab