Breakpoints in dynamically loaded add-ons don't seem to be installed correctly

[anevilyak]

If one sets a breakpoint in a function located in an add-on image, then exits the debugger and reloads the same app in it again, the breakpoint doesn't appear to get reinstalled correctly when the add-on is later loaded by the team. BreakpointManager::_UpdateBreakpoints() does get called appropriately on add-on load, and stepping through it indicates that the breakpoint was both found/created and successfully installed, but it simply doesn't seem to actually get hit during program execution.

[anevilyak]

The instance where I saw this by the way, the breakpoint would hypothetically have been hit almost immediately after add-on load: ​http://cgit.haiku-os.org/haiku/tree/src/kits/opengl/GLRendererRoster.cpp#n198 (the breakpoint was in the constructor of one of the add-ons in question, and would have been called via instantiate).

This does make me wonder if it's possible that this is simply a race between us receiving the image load notification, loading debug information and installing the breakpoints vs the thread executing past where the breakpoint would've been before we make it that far. I'm a little unclear on when execution does/doesn't get suspended when these debug events come into play though.

[bonefish]

Replying to anevilyak:

This does make me wonder if it's possible that this is simply a race between us receiving the image load notification, loading debug information and installing the breakpoints vs the thread executing past where the breakpoint would've been before we make it that far. I'm a little unclear on when execution does/doesn't get suspended when these debug events come into play though.

This is likely the cause. Have a look at how TeamDebugger::_HandleDebuggerMessage() handles B_DEBUGGER_MESSAGE_IMAGE_CREATED. After the LoadImageDebugInfoJob is scheduled the thread is continued immediately. If there's a breakpoint in this image, we'll have to keep the thread blocked until the image debug info has been loaded and the breakpoints have been installed.

[anevilyak]

Replying to bonefish:

This is likely the cause. Have a look at how TeamDebugger::_HandleDebuggerMessage() handles B_DEBUGGER_MESSAGE_IMAGE_CREATED. After the LoadImageDebugInfoJob is scheduled the thread is continued immediately. If there's a breakpoint in this image, we'll have to keep the thread blocked until the image debug info has been loaded and the breakpoints have been installed.

Aha...will dig into that, thanks!

[anevilyak]

Attached a proposed fix. If that looks reasonable, let me know and I'll commit.

[bonefish]

The patch looks OK, save for a few cases of somewhat unusual formatting. Neither for TeamDebugger::ImageInfoPendingThread::fNext nor for TeamDebugger::ImageInfoPendingThreadHashDefinition::ValueType there's a fixed column to be respected, so there really isn't any need to break them. If you want to align it with the other variables/type just indent those more. In case of TeamDebugger::fImageInfoPendingThreads there is a particular column to be respected, but if that isn't possible, the common solution is to keep the variable on the same line as the type with only a single space in between. If that doesn't fit the 80 columns line limit, the line can be broken (I'd indent the continuing line one additional level).

[anevilyak]

Thanks for looking it over! Applied with suggested changes in hrev44927.