Fix crash in InspectorWindow.

- In the case where retrieval of a memory block failed, InspectorWindow
didn't handle the notification. Consequently, it never removed itself as
a listener from the failed block, nor did it release its reference for
it. Consequently, if one attempted to retrieve data from the same block
again, walking the listener list would crash due to the already-deleted
entry in the list.

- The success case had the same problem with regards to not removing its
listener, but was masked by virtue of the inspector currently being the
only user of the memory block manager, so in the latter case the blocks
would be properly released/destroyed and the aforementioned walk would
never occur.

- Adjust locking a bit to ensure that manipulating the listener list
always happens with the team lock held.

- Style fixes.
This commit is contained in:
Rene Gollent 2013-06-25 17:37:10 -04:00
parent 2214cb57ee
commit b906e10a5d
3 changed files with 68 additions and 22 deletions

View File

@ -1690,12 +1690,12 @@ TeamDebugger::_HandleInspectAddress(target_addr_t address,
return;
}
if (!memoryBlock->HasListener(listener))
memoryBlock->AddListener(listener);
if (!memoryBlock->IsValid()) {
AutoLocker< ::Team> teamLocker(fTeam);
if (!memoryBlock->HasListener(listener))
memoryBlock->AddListener(listener);
TeamMemory* memory = fTeam->GetTeamMemory();
// schedule the job
status_t result;
@ -1703,7 +1703,10 @@ TeamDebugger::_HandleInspectAddress(target_addr_t address,
new(std::nothrow) RetrieveMemoryBlockJob(fTeam, memory,
memoryBlock),
this)) != B_OK) {
memoryBlock->NotifyDataRetrieved(result);
memoryBlock->ReleaseReference();
_NotifyUser("Inspect Address", "Failed to retrieve memory data: %s",
strerror(result));
}

View File

@ -27,8 +27,9 @@
enum {
MSG_NAVIGATE_PREVIOUS_BLOCK = 'npbl',
MSG_NAVIGATE_NEXT_BLOCK = 'npnl'
MSG_NAVIGATE_PREVIOUS_BLOCK = 'npbl',
MSG_NAVIGATE_NEXT_BLOCK = 'npnl',
MSG_MEMORY_BLOCK_RETRIEVED = 'mbre',
};
@ -52,8 +53,7 @@ InspectorWindow::InspectorWindow(::Team* team, UserInterfaceListener* listener,
InspectorWindow::~InspectorWindow()
{
if (fCurrentBlock != NULL)
{
if (fCurrentBlock != NULL) {
fCurrentBlock->RemoveListener(this);
fCurrentBlock->ReleaseReference();
}
@ -186,15 +186,14 @@ InspectorWindow::_Init()
void
InspectorWindow::MessageReceived(BMessage* msg)
InspectorWindow::MessageReceived(BMessage* message)
{
switch (msg->what) {
switch (message->what) {
case MSG_INSPECT_ADDRESS:
{
target_addr_t address = 0;
bool addressValid = false;
if (msg->FindUInt64("address", &address) != B_OK)
{
if (message->FindUInt64("address", &address) != B_OK) {
ExpressionParser parser;
parser.SetSupportHexInput(true);
const char* addressExpression = fAddressInput->Text();
@ -238,10 +237,9 @@ InspectorWindow::MessageReceived(BMessage* msg)
case MSG_NAVIGATE_PREVIOUS_BLOCK:
case MSG_NAVIGATE_NEXT_BLOCK:
{
if (fCurrentBlock != NULL)
{
if (fCurrentBlock != NULL) {
target_addr_t address = fCurrentBlock->BaseAddress();
if (msg->what == MSG_NAVIGATE_PREVIOUS_BLOCK)
if (message->what == MSG_NAVIGATE_PREVIOUS_BLOCK)
address -= fCurrentBlock->Size();
else
address += fCurrentBlock->Size();
@ -252,9 +250,44 @@ InspectorWindow::MessageReceived(BMessage* msg)
}
break;
}
case MSG_MEMORY_BLOCK_RETRIEVED:
{
TeamMemoryBlock* block = NULL;
status_t result;
if (message->FindPointer("block",
reinterpret_cast<void **>(&block)) != B_OK
|| message->FindInt32("result", &result) != B_OK) {
break;
}
{
AutoLocker< ::Team>(fTeam);
block->RemoveListener(this);
}
if (result == B_OK) {
fCurrentBlock = block;
fMemoryView->SetTargetAddress(block, fCurrentAddress);
fPreviousBlockButton->SetEnabled(true);
fNextBlockButton->SetEnabled(true);
} else {
BString errorMessage;
errorMessage.SetToFormat("Unable to read address 0x%" B_PRIx64
": %s", block->BaseAddress(), strerror(result));
BAlert* alert = new(std::nothrow) BAlert("Inspect address",
errorMessage.String(), "Close");
if (alert == NULL)
break;
alert->Go(NULL);
block->ReleaseReference();
}
break;
}
default:
{
BWindow::MessageReceived(msg);
BWindow::MessageReceived(message);
break;
}
}
@ -275,13 +308,21 @@ InspectorWindow::QuitRequested()
void
InspectorWindow::MemoryBlockRetrieved(TeamMemoryBlock* block)
{
AutoLocker<BLooper> lock(this);
if (lock.IsLocked()) {
fCurrentBlock = block;
fMemoryView->SetTargetAddress(block, fCurrentAddress);
fPreviousBlockButton->SetEnabled(true);
fNextBlockButton->SetEnabled(true);
}
BMessage message(MSG_MEMORY_BLOCK_RETRIEVED);
message.AddPointer("block", block);
message.AddInt32("result", B_OK);
PostMessage(&message);
}
void
InspectorWindow::MemoryBlockRetrievalFailed(TeamMemoryBlock* block,
status_t result)
{
BMessage message(MSG_MEMORY_BLOCK_RETRIEVED);
message.AddPointer("block", block);
message.AddInt32("result", result);
PostMessage(&message);
}

View File

@ -41,6 +41,8 @@ public:
// TeamMemoryBlock::Listener
virtual void MemoryBlockRetrieved(TeamMemoryBlock* block);
virtual void MemoryBlockRetrievalFailed(
TeamMemoryBlock* block, status_t result);
// MemoryView::Listener
virtual void TargetAddressChanged(target_addr_t address);