Bug 515839

Confirmed, selecting a window layout crashes on Qt6 but not Qt5.

It crashes here
> 0   libkritaresourcewidgets.21.0.0.dylib	       0x103dbc2e0 KisResourceItemChooser::activate(QModelIndex const&) + 336 (KisResourceItemChooser.cpp:515)

When the window layout is activated, the updates are blocked on the window layout chooser, it emits the resourceSelected signal, a bunch of other stuff happens,  and then eventually it unblocks the updates.
However, by the time it gets back to `d->updatesBlocked = false;`, `d` is null for some reason, so it crashes instead.

Created attachment 189530 [details]
Fix closing windows cause crash

I decided to look into this a bit today. Attached a patch with a fix, 

It is mainly this line in KisWindowLayoutResource.cpp that cause the crash, when closing the windows:

https://invent.kde.org/graphics/krita/-/blame/cb158acfc714301289a63bff7aa892eb8c4f1f23/libs/ui/KisWindowLayoutResource.cpp#L157

I shuffled some things around in  openNecessaryWindows(), closeUnneededWindows(). And moved closeUnneededWindows() to the end. This seems to fix the crash.

I can submit a PR if you want, or you can take my patch and investigate further.

Comment on attachment 189530 [details]
Fix closing windows cause crash

diff --git a/libs/ui/KisWindowLayoutResource.cpp b/libs/ui/KisWindowLayoutResource.cpp
index 2fcec19221..65ff208bf7 100644
--- a/libs/ui/KisWindowLayoutResource.cpp
+++ b/libs/ui/KisWindowLayoutResource.cpp
@@ -126,10 +126,8 @@ struct KisWindowLayoutResource::Private
                 mainWindow->show();
             }
         }
-    }
-
-    void closeUnneededWindows(QList<QPointer<KisMainWindow>> &currentWindows) {
-        QVector<QPointer<KisMainWindow>> windowsToClose;
+        
+        QVector<QPointer<KisMainWindow>> windowsToMigrate;
 
         Q_FOREACH(KisMainWindow *mainWindow, currentWindows) {
             bool keep = false;
@@ -141,8 +139,8 @@ struct KisWindowLayoutResource::Private
             }
 
             if (!keep) {
-                windowsToClose.append(mainWindow);
-
+                windowsToMigrate.append(mainWindow);
+                
                 // Set the window hidden to prevent "show image in all windows" feature from opening new views on it
                 // while we migrate views onto the remaining windows
                 if (mainWindow->isVisible()) {
@@ -150,11 +148,25 @@ struct KisWindowLayoutResource::Private
                 }
             }
         }
+        
+        migrateViewsFromClosingWindows(windowsToMigrate);
+    }
 
-        migrateViewsFromClosingWindows(windowsToClose);
+    void closeUnneededWindows(QList<QPointer<KisMainWindow>> &currentWindows) {
+        QVector<QPointer<KisMainWindow>> windowsToClose;
+
+        Q_FOREACH(KisMainWindow *mainWindow, currentWindows) {
+            bool keep = false;
+            Q_FOREACH(const Window &window, windows) {
+                if (window.windowId == mainWindow->id()) {
+                    keep = true;
+                    break;
+                }
+            }
 
-        Q_FOREACH(QPointer<KisMainWindow> mainWindow, windowsToClose) {
-            mainWindow->close();
+            if (!keep) {
+                mainWindow->close();
+            }
         }
     }
 
@@ -248,8 +260,9 @@ void KisWindowLayoutResource::applyLayout()
     layoutManager->setLastUsedLayout(this);
 
     QList<QPointer<KisMainWindow>> currentWindows = kisPart->mainWindows();
+    bool createWindows = d->windows.isEmpty();
 
-    if (d->windows.isEmpty()) {
+    if (createWindows) {
         // No windows defined (e.g. fresh new session). Leave things as they are, but make sure there's at least one visible main window
         if (kisPart->mainwindowCount() == 0) {
             kisPart->createMainWindow();
@@ -258,7 +271,6 @@ void KisWindowLayoutResource::applyLayout()
         }
     } else {
         d->openNecessaryWindows(currentWindows);
-        d->closeUnneededWindows(currentWindows);
     }
 
     // Wait for the windows to finish opening / closing before applying saved geometry.
@@ -300,6 +312,9 @@ void KisWindowLayoutResource::applyLayout()
 
     layoutManager->setShowImageInAllWindowsEnabled(d->showImageInAllWindows);
     layoutManager->setPrimaryWorkspaceFollowsFocus(d->primaryWorkspaceFollowsFocus, d->primaryWindow);
+    if (!createWindows) {
+        d->closeUnneededWindows(currentWindows);
+    }
 }
 
 bool KisWindowLayoutResource::saveToDevice(QIODevice *dev) const

Sorry, I am not used to this bug trackers function. I updated the patch, there was one typo, I uncommented a commented line for some reason.

Created attachment 189534 [details]
Fix closing windows cause crash (fixed typo)

Re-uploading patch.

Hey vurentjie, we use gitlab. dmitryK has moved your patch onto gitlab, but hasn't yet mentioned that here, so I am going so now: https://invent.kde.org/graphics/krita/-/merge_requests/2698

(In reply to wolthera from comment #6)
> Hey vurentjie, we use gitlab. dmitryK has moved your patch onto gitlab, but
> hasn't yet mentioned that here, so I am going so now:
> https://invent.kde.org/graphics/krita/-/merge_requests/2698

Thanks, the link you share is different, I looked now, and it is this merge request:
https://invent.kde.org/graphics/krita/-/merge_requests/2695

Ah, sorry, I had too many tabs open. Good to see you found it though,

The related MR: https://invent.kde.org/graphics/krita/-/merge_requests/2695

Hi, vurentjie!

The bug is now fixed in both, master and krita/6.0! Thank you for the report and the proposed fix! :)

https://invent.kde.org/graphics/krita/-/commit/5cf31d1445ef45373ffe9e5d08a74d8914085fcc