Skip to content

Commit d7e5879

Browse files
committed
[SharedCache]"Fix" possible deadlock when loading images from the UI
This disables the filter for already added images, there is another check when we go to apply so that it fails early but the real issue is the fact this happened at all. Within the Objective-C processor there is a call to `BeginUndoActions` which calls `ExecuteOnMainThreadAndWait`, while the objective-c processor is holding the lock to the controller. To fix this we really need to make the undo action system not call `ExecuteOnMainThreadAndWait` which is deadlock city, to do that really the only solution is to make the `UndoBuffer` thread-safe and remove the calls to execute undo related stuff on the main thread, this will make it so that when a call to `BeginUndoActions` there is no requirement to wait on pending undo actions as they are all submitted immediately.
1 parent 9bd77ec commit d7e5879

File tree

1 file changed

+11
-4
lines changed

1 file changed

+11
-4
lines changed

view/sharedcache/ui/dsctriage.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,17 @@ DSCTriageView::~DSCTriageView()
7474

7575

7676
void DSCTriageView::loadImagesWithAddr(const std::vector<uint64_t>& addresses, bool includeDependencies) {
77-
auto controller = SharedCacheController::GetController(*this->m_data);
77+
auto controller = SharedCacheController::GetController(*m_data);
7878
if (!controller)
7979
return;
8080

81+
// TODO: NOTE ABOUT `IsImageLoaded` BEING COMMENTED OUT. PLEASE READ.
82+
// TODO: Because commiting undo actions will use main thread to synchronize we must not be holding any locks
83+
// TODO: This can really only ever be removed if:
84+
// TODO: 1. we can set a user function type without creating an undo action, basically like the rest of shared cache
85+
// TODo: use an auto function or some hack to get the user function but without the undo action.
86+
// TODO: 2. we can use the undo buffer from any thread and not just the main thread
87+
// TODO: I have exhausted all other options, this is a serious issue we should address soon.
8188
typedef std::vector<CacheImage> ImageList;
8289
ImageList images = {};
8390
for (const uint64_t& addr : addresses)
@@ -86,8 +93,8 @@ void DSCTriageView::loadImagesWithAddr(const std::vector<uint64_t>& addresses, b
8693
if (image.has_value())
8794
{
8895
// Only try to load if we have not already.
89-
if (!controller->IsImageLoaded(*image))
90-
images.emplace_back(*image);
96+
// if (!controller->IsImageLoaded(*image))
97+
images.emplace_back(*image);
9198

9299
// TODO: We currently only add direct dependencies, may want to make the depth configurable?
93100
if (includeDependencies)
@@ -96,7 +103,7 @@ void DSCTriageView::loadImagesWithAddr(const std::vector<uint64_t>& addresses, b
96103
for (const auto& depName : dependencies)
97104
{
98105
auto depImage = controller->GetImageWithName(depName);
99-
if (depImage.has_value() && !controller->IsImageLoaded(*depImage))
106+
if (depImage.has_value()/* && !controller->IsImageLoaded(*depImage) */)
100107
{
101108
images.emplace_back(*depImage);
102109
}

0 commit comments

Comments
 (0)