mirror/aseprite
1
0
Fork 0
mirror of https://github.com/aseprite/aseprite.git synced 2025-02-19 15:40:31 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix deadlock destroying SpriteEvents disconnecting observers onCloseDocument

Browse Source 
Instead of using an onDestroy(Document), which can be called from the
ClosedDocs background thread, we can disconnect all Doc observers on a
new onCloseDocument() event called from Doc::close() (from the UI
thread).

This deadlock issue was commented here:
https://github.com/aseprite/aseprite/pull/3009#issue-1029413592
This commit is contained in:
David Capello 2021-10-19 12:43:56 -03:00
parent 246b1930f4
commit 4dcdcfe387
3 changed files with 26 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/app/doc.cpp
View File

@ -70,14 +70,6 @@ Doc::Doc(Sprite* sprite)
Doc::~Doc()
{
DOC_TRACE("DOC: Deleting", this);
try {
notify_observers<Doc*>(&DocObserver::onDestroy, this);
}
catch (...) {
LOG(ERROR, "DOC: Exception on DocObserver::onDestroy()\n");
}
removeFromContext();
}
@ -584,6 +576,13 @@ Doc* Doc::duplicate(DuplicateType type) const
void Doc::close()
{
try {
notify_observers<Doc*>(&DocObserver::onCloseDocument, this);
}
catch (...) {
LOG(ERROR, "DOC: Exception on DocObserver::onCloseDocument()\n");
}
removeFromContext();
}

4
src/app/doc_observer.h
View File

@ -1,5 +1,5 @@
// Aseprite
// Copyright (C) 2018-2020 Igara Studio S.A.
// Copyright (C) 2018-2021 Igara Studio S.A.
// Copyright (C) 2001-2018 David Capello
//
// This program is distributed under the terms of
@ -17,7 +17,7 @@ namespace app {
public:
virtual ~DocObserver() { }
virtual void onDestroy(Doc* doc) { }
virtual void onCloseDocument(Doc* doc) { }
virtual void onFileNameChanged(Doc* doc) { }
// General update. If an observer receives this event, it's because

27
src/app/script/events_class.cpp
View File

@ -170,11 +170,12 @@ public:
}
~SpriteEvents() {
if (m_observingUndo) {
doc()->undoHistory()->remove_observer(this);
m_observingUndo = false;
auto doc = this->doc();
ASSERT(doc);
if (doc) {
disconnectFromUndoHistory(doc);
doc->remove_observer(this);
}
doc()->remove_observer(this);
}
EventType eventType(const char* eventName) const {
@ -185,11 +186,13 @@ public:
}
// DocObserver impl
void onDestroy(Doc* doc) override {
void onCloseDocument(Doc* doc) override {
auto it = g_spriteEvents.find(m_spriteId);
ASSERT(it != g_spriteEvents.end());
if (it != g_spriteEvents.end())
if (it != g_spriteEvents.end()) {
// As this is an unique_ptr, here we are calling ~SpriteEvents()
g_spriteEvents.erase(it);
}
}
// DocUndoObserver impl
@ -211,10 +214,7 @@ private:
void onRemoveLastListener(EventType eventType) override {
switch (eventType) {
case Change: {
if (m_observingUndo) {
doc()->undoHistory()->remove_observer(this);
m_observingUndo = false;
}
disconnectFromUndoHistory(doc());
break;
}
}
@ -228,6 +228,13 @@ private:
return nullptr;
}
void disconnectFromUndoHistory(Doc* doc) {
if (m_observingUndo) {
doc->undoHistory()->remove_observer(this);
m_observingUndo = false;
}
}
ObjectId m_spriteId;
bool m_observingUndo = false;
};