Skip to content

Commit 34622ad

Browse files
committed
fix strings missing race condition
1 parent dbdf338 commit 34622ad

File tree

4 files changed

+179
-10
lines changed

4 files changed

+179
-10
lines changed

examples/triage/exports.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ GenericExportsModel::GenericExportsModel(QWidget* parent, BinaryViewRef data): Q
3232
connect(m_updateTimer, &QTimer::timeout, this, &GenericExportsModel::updateModel);
3333
connect(this, &GenericExportsModel::updateTimerOnUIThread, this, [=, this]() {
3434
updateTimer(m_needsUpdate);
35-
});
35+
}, Qt::QueuedConnection);
3636

3737
m_data->RegisterNotification(this);
3838

@@ -260,19 +260,26 @@ void GenericExportsModel::updateTimer(bool needsUpdate)
260260
void GenericExportsModel::pauseUpdates()
261261
{
262262
m_updatesPaused = true;
263+
m_dirtyWhilePaused = false;
263264
setNeedsUpdate(false);
264265
}
265266

266267
void GenericExportsModel::resumeUpdates()
267268
{
268269
m_updatesPaused = false;
269-
setNeedsUpdate(true);
270+
// Only refresh if we got notifications while paused
271+
if (m_dirtyWhilePaused.exchange(false))
272+
setNeedsUpdate(true);
270273
}
271274

272275
void GenericExportsModel::onBinaryViewNotification()
273276
{
274277
if (m_updatesPaused)
278+
{
279+
// Track that updates occurred while hidden
280+
m_dirtyWhilePaused = true;
275281
return;
282+
}
276283

277284
// This can be called from any thread so we cannot directly
278285
// update the timer. Emitting a signal is relatively expensive

examples/triage/exports.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ class GenericExportsModel : public QAbstractItemModel, public BinaryNinja::Binar
2222
std::atomic<bool> m_updatesPaused = false;
2323
// Read/written from arbitrary threads while processing notifications.
2424
std::atomic<bool> m_needsUpdate = true;
25+
// Tracks if notifications arrived while paused
26+
std::atomic<bool> m_dirtyWhilePaused = false;
2527

2628
void performSort(int col, Qt::SortOrder order);
2729
void updateModel();

examples/triage/strings.cpp

Lines changed: 133 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,29 @@
1111
#include "fontsettings.h"
1212

1313

14-
GenericStringsModel::GenericStringsModel(QWidget* parent, BinaryViewRef data) : QAbstractItemModel(parent)
14+
GenericStringsModel::GenericStringsModel(QWidget* parent, BinaryViewRef data) : QAbstractItemModel(parent), BinaryDataNotification(StringUpdates)
1515
{
1616
m_data = data;
1717
m_totalCols = 3;
1818
m_sortCol = 0;
1919
m_sortOrder = Qt::AscendingOrder;
20-
m_allEntries = data->GetStrings();
21-
m_entries = m_allEntries;
20+
21+
m_updateTimer = new QTimer(this);
22+
m_updateTimer->setInterval(500);
23+
connect(m_updateTimer, &QTimer::timeout, this, &GenericStringsModel::updateModel);
24+
connect(this, &GenericStringsModel::updateTimerOnUIThread, this, [=, this]() {
25+
updateTimer(m_needsUpdate);
26+
}, Qt::QueuedConnection);
27+
28+
m_data->RegisterNotification(this);
29+
30+
updateModel();
31+
}
32+
33+
34+
GenericStringsModel::~GenericStringsModel()
35+
{
36+
m_data->UnregisterNotification(this);
2237
}
2338

2439

@@ -168,22 +183,119 @@ void GenericStringsModel::sort(int col, Qt::SortOrder order)
168183
}
169184

170185

171-
void GenericStringsModel::setFilter(const std::string& filterText)
186+
void GenericStringsModel::applyFilter()
172187
{
173-
beginResetModel();
174188
m_entries.clear();
175189
for (auto& entry : m_allEntries)
176190
{
177191
auto s = stringRefToQString(entry).toStdString();
178-
179-
if (FilteredView::match(s, filterText))
192+
193+
if (FilteredView::match(s, m_filter))
180194
m_entries.push_back(entry);
181195
}
182196
performSort(m_sortCol, m_sortOrder);
197+
}
198+
199+
200+
void GenericStringsModel::setFilter(const std::string& filterText)
201+
{
202+
m_filter = filterText;
203+
beginResetModel();
204+
applyFilter();
183205
endResetModel();
184206
}
185207

186208

209+
void GenericStringsModel::updateModel()
210+
{
211+
if (!m_needsUpdate)
212+
return;
213+
214+
setNeedsUpdate(false);
215+
beginResetModel();
216+
m_allEntries = m_data->GetStrings();
217+
applyFilter();
218+
endResetModel();
219+
}
220+
221+
222+
void GenericStringsModel::setNeedsUpdate(bool needed)
223+
{
224+
if (m_needsUpdate.exchange(needed) == needed)
225+
return;
226+
227+
updateTimer(needed);
228+
}
229+
230+
231+
void GenericStringsModel::updateTimer(bool needsUpdate)
232+
{
233+
if (needsUpdate && !m_updateTimer->isActive())
234+
m_updateTimer->start();
235+
if (!needsUpdate && m_updateTimer->isActive())
236+
m_updateTimer->stop();
237+
}
238+
239+
240+
void GenericStringsModel::pauseUpdates()
241+
{
242+
m_updatesPaused = true;
243+
m_dirtyWhilePaused = false;
244+
setNeedsUpdate(false);
245+
}
246+
247+
248+
void GenericStringsModel::resumeUpdates()
249+
{
250+
m_updatesPaused = false;
251+
// Only refresh if we got notifications while paused
252+
if (m_dirtyWhilePaused.exchange(false))
253+
setNeedsUpdate(true);
254+
}
255+
256+
257+
void GenericStringsModel::onBinaryViewNotification()
258+
{
259+
if (m_updatesPaused)
260+
{
261+
// Track that updates occurred while hidden
262+
m_dirtyWhilePaused = true;
263+
return;
264+
}
265+
266+
// This can be called from any thread so we cannot directly
267+
// update the timer. Emitting a signal is relatively expensive
268+
// given how frequently we receive notifications, so we only
269+
// emit a signal if we didn't already need an update.
270+
if (!m_needsUpdate.exchange(true))
271+
emit updateTimerOnUIThread();
272+
}
273+
274+
275+
void GenericStringsModel::OnStringFound(BinaryNinja::BinaryView* view, BNStringType type, uint64_t offset, size_t len)
276+
{
277+
onBinaryViewNotification();
278+
}
279+
280+
281+
void GenericStringsModel::OnStringRemoved(BinaryNinja::BinaryView* view, BNStringType type, uint64_t offset, size_t len)
282+
{
283+
onBinaryViewNotification();
284+
}
285+
286+
287+
void GenericStringsModel::OnDerivedStringFound(BinaryNinja::BinaryView* view, const BinaryNinja::DerivedString& str)
288+
{
289+
onBinaryViewNotification();
290+
}
291+
292+
293+
void GenericStringsModel::OnDerivedStringRemoved(BinaryNinja::BinaryView* view, const BinaryNinja::DerivedString& str)
294+
{
295+
onBinaryViewNotification();
296+
}
297+
298+
187299
StringsTreeView::StringsTreeView(StringsWidget* parent, TriageView* view, BinaryViewRef data) : QTreeView(parent)
188300
{
189301
m_data = data;
@@ -362,6 +474,20 @@ void StringsTreeView::keyPressEvent(QKeyEvent* event)
362474
}
363475

364476

477+
void StringsTreeView::showEvent(QShowEvent* event)
478+
{
479+
QTreeView::showEvent(event);
480+
m_model->resumeUpdates();
481+
}
482+
483+
484+
void StringsTreeView::hideEvent(QHideEvent* event)
485+
{
486+
QTreeView::hideEvent(event);
487+
m_model->pauseUpdates();
488+
}
489+
490+
365491
StringsWidget::StringsWidget(QWidget* parent, TriageView* view, BinaryViewRef data) : QWidget(parent)
366492
{
367493
QVBoxLayout* layout = new QVBoxLayout();

examples/triage/strings.h

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,43 @@
11
#pragma once
22

33
#include <QtCore/QAbstractItemModel>
4+
#include <QtCore/QTimer>
45
#include <QtWidgets/QTreeView>
56
#include "filter.h"
67

78

8-
class GenericStringsModel : public QAbstractItemModel
9+
class GenericStringsModel : public QAbstractItemModel, public BinaryNinja::BinaryDataNotification
910
{
11+
Q_OBJECT
12+
1013
BinaryViewRef m_data;
1114
std::vector<BNStringReference> m_allEntries, m_entries;
1215
int m_totalCols, m_sortCol;
1316
Qt::SortOrder m_sortOrder;
17+
std::string m_filter;
18+
QTimer* m_updateTimer;
19+
20+
// Read from arbitrary threads while processing notifications.
21+
std::atomic<bool> m_updatesPaused = false;
22+
// Read/written from arbitrary threads while processing notifications.
23+
std::atomic<bool> m_needsUpdate = true;
24+
// Tracks if notifications arrived while paused
25+
std::atomic<bool> m_dirtyWhilePaused = false;
1426

1527
void performSort(int col, Qt::SortOrder order);
28+
void updateModel();
29+
void applyFilter();
30+
31+
void updateTimer(bool);
32+
void setNeedsUpdate(bool);
33+
void onBinaryViewNotification();
34+
35+
signals:
36+
void updateTimerOnUIThread();
1637

1738
public:
1839
GenericStringsModel(QWidget* parent, BinaryViewRef data);
40+
virtual ~GenericStringsModel();
1941

2042
virtual int columnCount(const QModelIndex& parent) const override;
2143
virtual int rowCount(const QModelIndex& parent) const override;
@@ -26,8 +48,16 @@ class GenericStringsModel : public QAbstractItemModel
2648
virtual void sort(int col, Qt::SortOrder order) override;
2749
void setFilter(const std::string& filterText);
2850

51+
void pauseUpdates();
52+
void resumeUpdates();
53+
2954
BNStringReference getStringRefAt(const QModelIndex& index) const;
3055
QString stringRefToQString(const BNStringReference& index) const;
56+
57+
virtual void OnStringFound(BinaryNinja::BinaryView* data, BNStringType type, uint64_t offset, size_t len) override;
58+
virtual void OnStringRemoved(BinaryNinja::BinaryView* data, BNStringType type, uint64_t offset, size_t len) override;
59+
virtual void OnDerivedStringFound(BinaryNinja::BinaryView* data, const BinaryNinja::DerivedString& str) override;
60+
virtual void OnDerivedStringRemoved(BinaryNinja::BinaryView* data, const BinaryNinja::DerivedString& str) override;
3161
};
3262

3363

@@ -36,6 +66,8 @@ class StringsWidget;
3666

3767
class StringsTreeView : public QTreeView, public FilterTarget
3868
{
69+
Q_OBJECT
70+
3971
BinaryViewRef m_data;
4072
StringsWidget* m_parent;
4173
TriageView* m_view;
@@ -59,6 +91,8 @@ class StringsTreeView : public QTreeView, public FilterTarget
5991
protected:
6092
virtual void keyPressEvent(QKeyEvent* event) override;
6193
virtual bool event(QEvent* event) override;
94+
virtual void showEvent(QShowEvent* event) override;
95+
virtual void hideEvent(QHideEvent* event) override;
6296

6397
private Q_SLOTS:
6498
void stringSelected(const QModelIndex& cur, const QModelIndex& prev);

0 commit comments

Comments
 (0)