[haiku-commits] haiku: hrev45791 - in src/apps/debugger: user_interface/gui/inspector_window controllers

  • From: anevilyak@xxxxxxxxx
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Tue, 25 Jun 2013 23:42:41 +0200 (CEST)

hrev45791 adds 1 changeset to branch 'master'
old head: 2214cb57eeed6481d73b5ef887b3ce27d47eaf8f
new head: b906e10a5d3681c1a23008f85ff01ee30086d439
overview: http://cgit.haiku-os.org/haiku/log/?qt=range&q=b906e10+%5E2214cb5

----------------------------------------------------------------------------

b906e10: 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.

                                      [ Rene Gollent <anevilyak@xxxxxxxxx> ]

----------------------------------------------------------------------------

Revision:    hrev45791
Commit:      b906e10a5d3681c1a23008f85ff01ee30086d439
URL:         http://cgit.haiku-os.org/haiku/commit/?id=b906e10
Author:      Rene Gollent <anevilyak@xxxxxxxxx>
Date:        Tue Jun 25 21:37:10 2013 UTC

----------------------------------------------------------------------------

3 files changed, 68 insertions(+), 22 deletions(-)
src/apps/debugger/controllers/TeamDebugger.cpp   |  9 ++-
.../gui/inspector_window/InspectorWindow.cpp     | 79 +++++++++++++++-----
.../gui/inspector_window/InspectorWindow.h       |  2 +

----------------------------------------------------------------------------

diff --git a/src/apps/debugger/controllers/TeamDebugger.cpp 
b/src/apps/debugger/controllers/TeamDebugger.cpp
index 7974368..6026df6 100644
--- a/src/apps/debugger/controllers/TeamDebugger.cpp
+++ b/src/apps/debugger/controllers/TeamDebugger.cpp
@@ -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));
                }
diff --git 
a/src/apps/debugger/user_interface/gui/inspector_window/InspectorWindow.cpp 
b/src/apps/debugger/user_interface/gui/inspector_window/InspectorWindow.cpp
index 1293e2f..333fa30 100644
--- a/src/apps/debugger/user_interface/gui/inspector_window/InspectorWindow.cpp
+++ b/src/apps/debugger/user_interface/gui/inspector_window/InspectorWindow.cpp
@@ -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);
 }
 
 
diff --git 
a/src/apps/debugger/user_interface/gui/inspector_window/InspectorWindow.h 
b/src/apps/debugger/user_interface/gui/inspector_window/InspectorWindow.h
index 320e344..528701d 100644
--- a/src/apps/debugger/user_interface/gui/inspector_window/InspectorWindow.h
+++ b/src/apps/debugger/user_interface/gui/inspector_window/InspectorWindow.h
@@ -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);


Other related posts:

  • » [haiku-commits] haiku: hrev45791 - in src/apps/debugger: user_interface/gui/inspector_window controllers - anevilyak