fix file manager threaded instance destroy bug

apps/file_manager/commands.cpp

@@ -56,7 +56,9 @@ void CommandRename(Instance *instance, EsElement *, EsCommand *) {
 					},
 
 					.then = [] (Instance *instance, Task *task) {
-						if (task->result != ES_SUCCESS) {
+						if (instance->closed) {
+							// Ignore.
+						} else if (task->result != ES_SUCCESS) {
 							InstanceReportError(instance, ERROR_RENAME_ITEM, task->result);
 						} else {
 							Folder *folder = instance->folder;
@@ -105,7 +107,9 @@ void CommandNewFolder(Instance *instance, EsElement *, EsCommand *) {
 		},
 
 		.then = [] (Instance *instance, Task *task) {
-			if (task->result != ES_SUCCESS) {
+			if (instance->closed) {
+				// Ignore.
+			} else if (task->result != ES_SUCCESS) {
 				InstanceReportError(instance, ERROR_NEW_FOLDER, task->result);
 			} else {
 				Folder *folder = instance->folder;
@@ -405,7 +409,7 @@ void CommandPasteTask(EsUserTask *userTask, EsGeneric _task) {
 	for (uintptr_t i = 0; i < instances.Length(); i++) {
 		Instance *instance = instances[i];
 
-		if (instance->issuedPasteTask == task) {
+		if (!instance->closed && instance->issuedPasteTask == task) {
 			instance->issuedPasteTask = nullptr;
 
 			if (error != ES_SUCCESS) {

apps/file_manager/folder.cpp

@@ -77,6 +77,9 @@ EsError FSDirEnumerate(Folder *folder) {
 	EsDirectoryChild *buffer = nullptr;
 	ptrdiff_t _entryCount = EsDirectoryEnumerateChildren(STRING(folder->path), &buffer);
 
+	// TODO.
+	EsSleep(5000);
+
 	if (!ES_CHECK_ERROR(_entryCount)) {
 		for (intptr_t i = 0; i < _entryCount; i++) {
 			FolderAddEntry(folder, buffer[i].name, buffer[i].nameBytes, &buffer[i]);
@@ -301,6 +304,7 @@ void BookmarkAdd(String path, bool saveConfiguration = true) {
 	if (saveConfiguration) ConfigurationSave();
 
 	for (uintptr_t i = 0; i < instances.Length(); i++) {
+		if (instances[i]->closed) continue;
 		EsListViewInsert(instances[i]->placesView, PLACES_VIEW_GROUP_BOOKMARKS, bookmarks.Length(), 1);
 	}
 }
@@ -311,6 +315,7 @@ void BookmarkRemove(String path) {
 			bookmarks.Delete(i);
 
 			for (uintptr_t i = 0; i < instances.Length(); i++) {
+				if (instances[i]->closed) continue;
 				EsListViewRemove(instances[i]->placesView, PLACES_VIEW_GROUP_BOOKMARKS, i + 1, 1);
 			}
 
@@ -433,6 +438,7 @@ void FolderAddEntryAndUpdateInstances(Folder *folder, const char *name, size_t n
 
 	for (uintptr_t i = 0; i < folder->attachedInstances.Length(); i++) {
 		Instance *instance = folder->attachedInstances[i];
+		if (instance->closed) continue;
 		listEntry.selected = instance == selectItem;
 		InstanceAddSingle(instance, listEntry);
 		InstanceUpdateStatusString(instance);
@@ -445,6 +451,7 @@ uint64_t FolderRemoveEntryAndUpdateInstances(Folder *folder, const char *name, s
 
 	if (entry) {
 		for (uintptr_t i = 0; i < folder->attachedInstances.Length(); i++) {
+			if (folder->attachedInstances[i]->closed) continue;
 			InstanceRemoveSingle(folder->attachedInstances[i], entry);
 		}
 
@@ -489,6 +496,7 @@ void FolderPathMoved(String oldPath, String newPath, bool saveConfiguration) {
 
 		if (PathReplacePrefix(&folder->path, oldPath, newPath)) {
 			for (uintptr_t i = 0; i < folder->attachedInstances.Length(); i++) {
+				if (folder->attachedInstances[i]->closed) continue;
 				InstanceFolderPathChanged(folder->attachedInstances[i], false);
 			}
 		}
@@ -503,6 +511,7 @@ void FolderPathMoved(String oldPath, String newPath, bool saveConfiguration) {
 	}
 
 	for (uintptr_t i = 0; i < instances.Length(); i++) {
+		if (instances[i]->closed) continue;
 		EsListViewInvalidateAll(instances[i]->placesView);
 
 		for (uintptr_t j = 0; j < instances[i]->pathBackwardHistory.Length(); j++) {
@@ -581,6 +590,7 @@ void FolderRefresh(Folder *folder) {
 	Array<Instance *> instancesToRefresh = {};
 
 	for (uintptr_t i = 0; i < folder->attachedInstances.Length(); i++) {
+		if (folder->attachedInstances[i]->closed) continue;
 		instancesToRefresh.Add(folder->attachedInstances[i]);
 	}
 

apps/file_manager/main.cpp

@@ -179,6 +179,8 @@ struct Instance : EsInstance {
 	Task blockingTask;
 	volatile bool blockingTaskInProgress, blockingTaskReachedTimeout;
 	volatile uint32_t blockingTaskID;
+
+	bool closed; // The window has been destroyed (or is about to be).
 };
 
 struct NamespaceHandler {
@@ -205,7 +207,7 @@ struct Folder {
 	HashTable entries;
 	EsArena entryArena;
 
-	Array<Instance *> attachedInstances;
+	Array<Instance *> attachedInstances; // NOTE Check Instance::closed is false before accessing the UI!
 	uintptr_t referenceCount;
 
 	String path;
@@ -245,7 +247,7 @@ void ThumbnailGenerateIfNeeded(Folder *folder, FolderEntry *entry, bool fromFold
 Array<Drive> drives;
 EsMutex drivesMutex;
 
-Array<Instance *> instances;
+Array<Instance *> instances; // NOTE Check Instance::closed is false before accessing the UI!
 
 Array<String> bookmarks;
 #define FOLDER_VIEW_SETTINGS_MAXIMUM_ENTRIES (10000)
@@ -330,11 +332,15 @@ void BlockingTaskComplete(Instance *instance) {
 	if (task->then) {
 		task->then(instance, task);
 	}
+
+	EsInstanceCloseReference(instance);
 }
 
 void BlockingTaskQueue(Instance *instance, Task task) {
 	EsAssert(!instance->blockingTaskInProgress); // Cannot queue a new blocking task if the previous has not finished.
 
+	EsInstanceOpenReference(instance); // Don't destroy the instance while the task is in progress.
+
 	instance->blockingTask = task;
 	instance->blockingTaskInProgress = true;
 
@@ -434,6 +440,7 @@ void DriveRemove(const char *prefix, size_t prefixBytes) {
 			found = true;
 
 			for (uintptr_t i = 0; i < instances.Length(); i++) {
+				if (instances[i]->closed) continue;
 				EsListViewRemove(instances[i]->placesView, PLACES_VIEW_GROUP_DRIVES, index, 1);
 			}
 
@@ -459,6 +466,7 @@ void DriveAdd(const char *prefix, size_t prefixBytes) {
 	drives.Add(drive);
 
 	for (uintptr_t i = 0; i < instances.Length(); i++) {
+		if (instances[i]->closed) continue;
 		EsListViewInsert(instances[i]->placesView, PLACES_VIEW_GROUP_DRIVES, drives.Length(), 1);
 	}
 
@@ -520,6 +528,16 @@ void _start() {
 				instances.Add(instance);
 				InstanceCreateUI(instance);
 			}
+		} else if (message->type == ES_MSG_INSTANCE_CLOSE) {
+			Instance *instance = message->instanceClose.instance;
+			EsAssert(!instance->closed);
+			instance->closed = true;
+			instance->list = nullptr;
+			instance->placesView = nullptr;
+			instance->breadcrumbBar = nullptr;
+			instance->newFolderButton = nullptr;
+			instance->status = nullptr;
+			instance->blockingDialog = nullptr;
 		} else if (message->type == ES_MSG_INSTANCE_DESTROY) {
 			Instance *instance = message->instanceDestroy.instance;
 			EsApplicationStartupRequest request = EsInstanceGetStartupRequest(instance);
@@ -628,6 +646,7 @@ void _start() {
 			}
 
 			for (uintptr_t i = 0; i < instances.Length(); i++) {
+				if (instances[i]->closed) continue;
 				EsListViewInvalidateAll(instances[i]->list);
 			}
 

apps/file_manager/ui.cpp

@@ -72,6 +72,10 @@ bool InstanceLoadFolder(Instance *instance, String path /* takes ownership */, i
 	task.then = [] (Instance *instance, Task *task) {
 		// TODO Check if folder was marked for refresh.
 
+		if (instance->closed) {
+			return;
+		}
+
 		int historyMode = task->context.i & 0xFF;
 		int flags = task->context.i;
 		Folder *folder = instance->folder;
@@ -627,7 +631,7 @@ void ThumbnailGenerateTaskComplete(Instance *, Task *task) {
 	String parent = PathGetParent(task->string);
 
 	for (uintptr_t i = 0; i < instances.Length(); i++) {
-		if (StringEquals(parent, instances[i]->path)) { // TODO Support recursive views.
+		if (!instances[i]->closed && StringEquals(parent, instances[i]->path)) { // TODO Support recursive views.
 			EsListViewInvalidateAll(instances[i]->list);
 		}
 	}

desktop/api.cpp

@@ -670,7 +670,7 @@ void InstanceSave(EsInstance *_instance) {
 
 void InstanceClose(EsInstance *instance) {
 	if (!EsCommandByID(instance, ES_COMMAND_SAVE)->enabled) {
-		EsInstanceDestroy(instance);
+		EsInstanceClose(instance);
 		return;
 	}
 
@@ -717,7 +717,7 @@ void InstanceClose(EsInstance *instance) {
 	EsDialogAddButton(dialog, ES_FLAGS_DEFAULT, ES_STYLE_PUSH_BUTTON_DANGEROUS, INTERFACE_STRING(FileCloseWithModificationsDelete), 
 			[] (EsInstance *instance, EsElement *, EsCommand *) { 
 		EsDialogClose(instance->window->dialogs.Last()); 
-		EsInstanceDestroy(instance);
+		EsInstanceClose(instance);
 	});
 
 	EsButton *button = EsDialogAddButton(dialog, ES_BUTTON_DEFAULT, 0, INTERFACE_STRING(FileCloseWithModificationsSave), 
@@ -936,20 +936,12 @@ void EsInstanceCloseReference(EsInstance *_instance) {
 	}
 }
 
-void EsInstanceDestroy(EsInstance *instance) {
+void EsInstanceClose(EsInstance *instance) {
 	EsMessageMutexCheck();
-	InspectorWindow **inspector = &((APIInstance *) instance->_private)->attachedInspector;
+	EsMessage m = {};
-
+	m.type = ES_MSG_INSTANCE_CLOSE;
-	if (*inspector) {
+	m.instanceClose.instance = instance;
-		EsInstanceDestroy(*inspector);
+	EsMessagePost(nullptr, &m); 
-		(*inspector)->window->InternalDestroy();
-		*inspector = nullptr;
-	}
-
-	UndoManagerDestroy(instance->undoManager);
-	EsAssert(instance->window->instance == instance);
-	EsElementDestroy(instance->window);
-	EsInstanceCloseReference(instance);
 }
 
 EsWindow *WindowFromWindowID(EsObjectID id) {
@@ -1063,6 +1055,24 @@ EsMessage *EsMessageReceive() {
 			if (instance->fileStore) FileStoreCloseHandle(instance->fileStore);
 			EsHeapFree(instance);
 			EsHeapFree(message.message.instanceDestroy.instance);
+		} else if (message.message.type == ES_MSG_INSTANCE_CLOSE) {
+			EsInstance *instance = message.message.instanceClose.instance;
+			InspectorWindow **inspector = &((APIInstance *) instance->_private)->attachedInspector;
+
+			if (*inspector) {
+				EsInstance *instance2 = *inspector;
+				UndoManagerDestroy(instance2->undoManager);
+				EsAssert(instance2->window->instance == instance2);
+				EsElementDestroy(instance2->window);
+				EsInstanceCloseReference(instance2);
+				instance2->window->InternalDestroy();
+				*inspector = nullptr;
+			}
+
+			UndoManagerDestroy(instance->undoManager);
+			EsAssert(instance->window->instance == instance);
+			EsElementDestroy(instance->window);
+			EsInstanceCloseReference(instance);
 		} else if (message.message.type == ES_MSG_UNREGISTER_FILE_SYSTEM) {
 			for (uintptr_t i = 0; i < api.mountPoints.Length(); i++) {
 				if (api.mountPoints[i].information.id == message.message.unregisterFileSystem.id) {
@@ -1312,7 +1322,7 @@ EsMessage *EsMessageReceive() {
 					return &message.message;
 				}
 			}
-		} else if (type == ES_MSG_INSTANCE_DESTROY) {
+		} else if (type == ES_MSG_INSTANCE_DESTROY || type == ES_MSG_INSTANCE_CLOSE) {
 			APIInstance *instance = (APIInstance *) message.message.instanceDestroy.instance->_private;
 
 			if (!instance->internalOnly) {
@@ -1404,7 +1414,7 @@ void EsInstanceSaveComplete(EsMessage *message, bool success) {
 			EsCommandSetDisabled(&apiInstance->commandShowInFileManager, false);
 
 			if (apiInstance->closeAfterSaveCompletes) {
-				EsInstanceDestroy(instance);
+				EsInstanceClose(instance);
 			}
 		} else {
 			apiInstance->closeAfterSaveCompletes = false;

desktop/desktop.cpp

@@ -1524,7 +1524,7 @@ void InstanceBlankTabCreate(EsMessage *message) {
 
 			if (ApplicationInstanceStart(((InstalledApplication *) element->userData.p)->id, nullptr, instance)) {
 				WindowTabActivate(instance->tab, true);
-				EsInstanceDestroy(element->instance);
+				EsInstanceClose(element->instance);
 			}
 		});
 	}

desktop/os.header

@@ -1040,6 +1040,7 @@ enum EsMessageType {
 	ES_MSG_INSTANCE_OPEN			= 0x7003
 	ES_MSG_INSTANCE_SAVE			= 0x7004
 	ES_MSG_INSTANCE_DESTROY			= 0x7005
+	ES_MSG_INSTANCE_CLOSE			= 0x7006
 
 	// User messages:
 	ES_MSG_USER_START			= 0x8000
@@ -1744,6 +1745,10 @@ struct EsMessageInstanceDestroy {
 	ES_INSTANCE_TYPE *instance;
 };
 
+struct EsMessageInstanceClose {
+	ES_INSTANCE_TYPE *instance;
+};
+
 // Internal system messages.
 
 struct EsMessageProcessCrash {
@@ -1856,6 +1861,7 @@ struct EsMessage {
 		EsMessageInstanceOpen instanceOpen;
 		EsMessageInstanceSave instanceSave;
 		EsMessageInstanceDestroy instanceDestroy;
+		EsMessageInstanceClose instanceClose;
 
 		// Internal messages:
 		void *_argument;
@@ -2366,9 +2372,9 @@ function void EsCommandSetCallback(EsCommand *command, EsCommandCallback callbac
 function void EsCommandSetDisabled(EsCommand *command, bool disabled);
 function void EsCommandSetCheck(EsCommand *command, EsCheckState check, bool sendUpdatedMessage);
 
-function void EsInstanceOpenReference(EsInstance *_instance);
+function void EsInstanceOpenReference(ES_INSTANCE_TYPE *_instance);
-function void EsInstanceCloseReference(EsInstance *_instance);
+function void EsInstanceCloseReference(ES_INSTANCE_TYPE *_instance); // Sends ES_MSG_INSTANCE_DESTROY when all references closed.
-function void EsInstanceDestroy(ES_INSTANCE_TYPE *instance);
+function void EsInstanceClose(ES_INSTANCE_TYPE *instance); // Sends ES_MSG_INSTANCE_CLOSE, and closes the window's reference to the instance.
 function void EsInstanceSetActiveUndoManager(ES_INSTANCE_TYPE *instance, EsUndoManager *manager);
 function void EsInstanceSetClassEditor(ES_INSTANCE_TYPE *instance, const EsInstanceClassEditorSettings *settings);
 function void EsInstanceSetClassViewer(ES_INSTANCE_TYPE *instance, const EsInstanceClassViewerSettings *settings);

util/api_table.ini

@@ -136,6 +136,7 @@ EsBufferFormat=134
 EsEventReset=135
 EsEventSet=136
 EsInstanceCloseReference=137
+EsInstanceClose=138
 EsMutexAcquire=140
 EsMutexDestroy=141
 EsMutexRelease=142
@@ -296,7 +297,6 @@ EsSliderCreate=296
 EsSystemConfigurationReadString=297
 EsSliderGetValue=298
 EsGameControllerStatePoll=299
-EsInstanceDestroy=300
 EsRandomU8=301
 EsRandomU64=302
 EsConnectionPoll=303