Debugger: Fix #13939, more work on #13800.

- Fix various cases where OpenHashTables weren't being cleared properly.
- Fix various reference counting errors.
- Simplify FileManager reference handling.
- Fix bug in LocatableDirectory where the directory named '/' would have its
  name returned as empty. This would lead to failed lookups for entries already
  in the table, and ultimately corrupted the hash table when deleting unused
  entries, leading to #13939. This was previously never noticed due to the
  entries not being freed properly.
- AbbreviationTable wasn't clearing its entries.
This commit is contained in:
Rene Gollent
2018-01-10 19:29:36 -05:00
parent e56d7050a3
commit 3995592cdf
16 changed files with 56 additions and 35 deletions
@@ -261,4 +261,5 @@ VariablesViewState::_Cleanup()
} }
SetValues(NULL); SetValues(NULL);
SetExpressionValues(NULL);
} }
@@ -2175,7 +2175,7 @@ SourceView::SetStackTrace(StackTrace* stackTrace, Thread* activeThread)
{ {
TRACE_GUI("SourceView::SetStackTrace(%p)\n", stackTrace); TRACE_GUI("SourceView::SetStackTrace(%p)\n", stackTrace);
if (stackTrace == fStackTrace) if (stackTrace == fStackTrace && activeThread == fActiveThread)
return; return;
if (fActiveThread != NULL) if (fActiveThread != NULL)
@@ -1324,6 +1324,7 @@ VariablesView::VariableTableModel::ValueNodeValueChanged(ValueNode* valueNode)
if (error != B_OK) if (error != B_OK)
return; return;
BReference<TableCellValueRenderer> rendererReference(renderer, true);
// set value/handler/renderer // set value/handler/renderer
modelNode->SetValue(value); modelNode->SetValue(value);
modelNode->SetValueHandler(valueHandler); modelNode->SetValueHandler(valueHandler);
@@ -1337,8 +1338,6 @@ VariablesView::VariableTableModel::ValueNodeValueChanged(ValueNode* valueNode)
settings->RestoreValues(modelNode->GetLastRendererSettings()); settings->RestoreValues(modelNode->GetLastRendererSettings());
} }
// notify table model listeners // notify table model listeners
NotifyNodeChanged(modelNode); NotifyNodeChanged(modelNode);
} }
@@ -1803,6 +1802,17 @@ VariablesView::~VariablesView()
if (fTemporaryExpression != NULL) if (fTemporaryExpression != NULL)
fTemporaryExpression->ReleaseReference(); fTemporaryExpression->ReleaseReference();
if (fExpressions != NULL) {
ExpressionInfoEntry* entry = fExpressions->Clear();
while (entry != NULL) {
ExpressionInfoEntry* next = entry->next;
delete entry;
entry = next;
}
}
delete fExpressions;
} }
@@ -2246,6 +2256,7 @@ VariablesView::MessageReceived(BMessage* message)
{ {
ModelNode* node; ModelNode* node;
if (message->FindPointer("node", (void**)&node) == B_OK) { if (message->FindPointer("node", (void**)&node) == B_OK) {
BReference<ModelNode> nodeReference(node, true);
TreeTablePath path; TreeTablePath path;
if (fVariableTableModel->GetTreePath(node, path)) { if (fVariableTableModel->GetTreePath(node, path)) {
FunctionID* functionID = fStackFrame->Function() FunctionID* functionID = fStackFrame->Function()
@@ -343,10 +343,9 @@ IntegerValueHandler::AddIntegerFormatOption(OptionsSettingImpl* setting,
const char* id, const char* name, integer_format format) const char* id, const char* name, integer_format format)
{ {
FormatOption* option = new(std::nothrow) FormatOption(id, name, format); FormatOption* option = new(std::nothrow) FormatOption(id, name, format);
if (option == NULL || !setting->AddOption(option)) { BReference<FormatOption> optionReference(option, true);
delete option; if (option == NULL || !setting->AddOption(option))
return B_NO_MEMORY; return B_NO_MEMORY;
}
return B_OK; return B_OK;
} }
@@ -1111,7 +1111,7 @@ DwarfImageDebugInfo::_GetSourceFileIndex(CompilationUnit* unit,
for (int32 i = 0; const char* fileName = unit->FileAt(i, &directory); i++) { for (int32 i = 0; const char* fileName = unit->FileAt(i, &directory); i++) {
LocatableFile* file = fFileManager->GetSourceFile(directory, fileName); LocatableFile* file = fFileManager->GetSourceFile(directory, fileName);
if (file != NULL) { if (file != NULL) {
file->ReleaseReference(); BReference<LocatableFile> fileReference(file, true);
if (file == sourceFile) { if (file == sourceFile) {
return i + 1; return i + 1;
// indices are one-based // indices are one-based
@@ -52,6 +52,9 @@ ValueNodeManager::SetStackFrame(Thread* thread,
fStackFrame = stackFrame; fStackFrame = stackFrame;
fThread = thread; fThread = thread;
if (fStackFrame == NULL)
return B_OK;
fContainer = new(std::nothrow) ValueNodeContainer; fContainer = new(std::nothrow) ValueNodeContainer;
if (fContainer == NULL) if (fContainer == NULL)
return B_NO_MEMORY; return B_NO_MEMORY;
@@ -21,6 +21,12 @@ AbbreviationTable::AbbreviationTable(off_t offset)
AbbreviationTable::~AbbreviationTable() AbbreviationTable::~AbbreviationTable()
{ {
AbbreviationTableEntry* entry = fEntryTable.Clear(true);
while (entry != NULL) {
AbbreviationTableEntry* next = entry->next;
delete entry;
entry = next;
}
} }
+7 -21
View File
@@ -124,9 +124,6 @@ public:
entry->ReleaseReference(); entry->ReleaseReference();
entry = next; entry = next;
} }
while ((entry = fDeadEntries.RemoveHead()) != NULL)
entry->ReleaseReference();
} }
status_t Init() status_t Init()
@@ -202,21 +199,10 @@ private:
AutoLocker<FileManager> lock(fManager); AutoLocker<FileManager> lock(fManager);
if (fEntries.Lookup(EntryPath(entry)) == entry) if (fEntries.Lookup(EntryPath(entry)) == entry)
fEntries.Remove(entry); fEntries.Remove(entry);
else {
DeadEntryList::Iterator iterator = fDeadEntries.GetIterator();
while (iterator.HasNext()) {
if (iterator.Next() == entry) {
fDeadEntries.Remove(entry);
break;
}
}
}
LocatableDirectory* parent = entry->Parent(); LocatableDirectory* parent = entry->Parent();
if (parent != NULL) if (parent != NULL)
parent->RemoveEntry(entry); parent->RemoveEntry(entry);
delete entry;
} }
bool _LocateDirectory(LocatableDirectory* directory, bool _LocateDirectory(LocatableDirectory* directory,
@@ -306,23 +292,23 @@ private:
LocatableFile* _GetFile(const BString& directoryPath, const BString& name) LocatableFile* _GetFile(const BString& directoryPath, const BString& name)
{ {
BString normalizedDirPath;
_NormalizePath(directoryPath, normalizedDirPath);
// if already known return the file // if already known return the file
LocatableEntry* entry = _LookupEntry(EntryPath(directoryPath, name)); LocatableEntry* entry = _LookupEntry(EntryPath(normalizedDirPath, name));
if (entry != NULL) { if (entry != NULL) {
LocatableFile* file = dynamic_cast<LocatableFile*>(entry); LocatableFile* file = dynamic_cast<LocatableFile*>(entry);
if (file == NULL) if (file == NULL)
return NULL; return NULL;
if (file->AcquireReference() == 0) { if (file->AcquireReference() == 0)
fEntries.Remove(file); fEntries.Remove(file);
fDeadEntries.Insert(file); else
} else
return file; return file;
} }
// no such file yet -- create it // no such file yet -- create it
BString normalizedDirPath;
_NormalizePath(directoryPath, normalizedDirPath);
LocatableDirectory* directory = _GetDirectory(normalizedDirPath); LocatableDirectory* directory = _GetDirectory(normalizedDirPath);
if (directory == NULL) if (directory == NULL)
return NULL; return NULL;
@@ -337,6 +323,7 @@ private:
directory->AddEntry(file); directory->AddEntry(file);
fEntries.Insert(file); fEntries.Insert(file);
return file; return file;
} }
@@ -488,7 +475,6 @@ private:
private: private:
FileManager* fManager; FileManager* fManager;
LocatableEntryTable fEntries; LocatableEntryTable fEntries;
DeadEntryList fDeadEntries;
bool fIsLocal; bool fIsLocal;
}; };
-2
View File
@@ -65,7 +65,6 @@ private:
struct SourceFileHashDefinition; struct SourceFileHashDefinition;
typedef BOpenHashTable<EntryHashDefinition> LocatableEntryTable; typedef BOpenHashTable<EntryHashDefinition> LocatableEntryTable;
typedef DoublyLinkedList<LocatableEntry> DeadEntryList;
typedef BOpenHashTable<SourceFileHashDefinition> SourceFileTable; typedef BOpenHashTable<SourceFileHashDefinition> SourceFileTable;
typedef std::map<BString, BString> LocatedFileMap; typedef std::map<BString, BString> LocatedFileMap;
@@ -83,7 +82,6 @@ private:
Domain* fTargetDomain; Domain* fTargetDomain;
Domain* fSourceDomain; Domain* fSourceDomain;
SourceFileTable* fSourceFiles; SourceFileTable* fSourceFiles;
LocatedFileMap fSourceLocationMappings; LocatedFileMap fSourceLocationMappings;
}; };
@@ -24,6 +24,9 @@ LocatableDirectory::~LocatableDirectory()
const char* const char*
LocatableDirectory::Name() const LocatableDirectory::Name() const
{ {
if (fPath.Length() <= 1)
return fPath;
int32 lastSlash = fPath.FindLast('/'); int32 lastSlash = fPath.FindLast('/');
// return -1, if not found // return -1, if not found
return fPath.String() + (lastSlash + 1); return fPath.String() + (lastSlash + 1);
@@ -44,4 +44,5 @@ void
LocatableEntry::LastReferenceReleased() LocatableEntry::LastReferenceReleased()
{ {
fOwner->LocatableEntryUnused(this); fOwner->LocatableEntryUnused(this);
delete this;
} }
+6
View File
@@ -54,6 +54,12 @@ StackFrame::~StackFrame()
fDebugInfo->ReleaseReference(); fDebugInfo->ReleaseReference();
fCpuState->ReleaseReference(); fCpuState->ReleaseReference();
if (fValues != NULL)
fValues->ReleaseReference();
if (fValueInfos != NULL)
fValueInfos->ReleaseReference();
} }
@@ -44,6 +44,8 @@ TeamSettings::TeamSettings(const TeamSettings& other)
TeamSettings::~TeamSettings() TeamSettings::~TeamSettings()
{ {
_Unset(); _Unset();
delete fFileManagerSettings;
delete fSignalSettings;
} }
@@ -69,6 +69,7 @@ ValueNodeContainer::RemoveChild(ValueNodeChild* child)
if (child->Container() != this || !fChildren.RemoveItem(child)) if (child->Container() != this || !fChildren.RemoveItem(child))
return; return;
child->SetNode(NULL);
child->SetContainer(NULL); child->SetContainer(NULL);
child->ReleaseReference(); child->ReleaseReference();
} }
@@ -78,6 +79,7 @@ void
ValueNodeContainer::RemoveAllChildren() ValueNodeContainer::RemoveAllChildren()
{ {
for (int32 i = 0; ValueNodeChild* child = ChildAt(i); i++) { for (int32 i = 0; ValueNodeChild* child = ChildAt(i); i++) {
child->SetNode(NULL);
child->SetContainer(NULL); child->SetContainer(NULL);
child->ReleaseReference(); child->ReleaseReference();
} }
@@ -213,8 +213,6 @@ BListValueNode::ResolvedLocationAndValue(ValueLoader* valueLoader,
// load the value data // load the value data
status_t error = B_OK; status_t error = B_OK;
_location = location;
_value = NULL;
ValueLocation* memberLocation = NULL; ValueLocation* memberLocation = NULL;
CompoundType* baseType = dynamic_cast<CompoundType*>(fType); CompoundType* baseType = dynamic_cast<CompoundType*>(fType);
@@ -290,6 +288,10 @@ BListValueNode::ResolvedLocationAndValue(ValueLoader* valueLoader,
memberLocation = NULL; memberLocation = NULL;
} }
location->AcquireReference();
_location = location;
_value = NULL;
return B_OK; return B_OK;
} }
@@ -164,9 +164,6 @@ BMessageValueNode::ResolvedLocationAndValue(ValueLoader* valueLoader,
// load the value data // load the value data
status_t error = B_OK; status_t error = B_OK;
_location = location;
_value = NULL;
ValueLocation* memberLocation = NULL; ValueLocation* memberLocation = NULL;
BVariant headerAddress; BVariant headerAddress;
@@ -318,6 +315,10 @@ BMessageValueNode::ResolvedLocationAndValue(ValueLoader* valueLoader,
if (error != B_OK) if (error != B_OK)
return error; return error;
location->AcquireReference();
_location = location;
_value = NULL;
return B_OK; return B_OK;
} }