Regression: Crash when saving after pressing ctrl+S twice. #88570

System Information
Operating system: Linux-5.12.3-arch1-1-x86_64-with-glibc2.33 64 Bits
Graphics card: AMD Radeon RX 5700 (NAVI10, DRM 3.40.0, 5.12.3-arch1-1, LLVM 11.1.0) AMD 4.6 (Core Profile) Mesa 21.1.0

Blender Version
Broken: version: 2.92, 2.93.0 Beta (1a69d491e5), 3.0 Alpha (067587e329).
Worked: 2.83

Caused by 796412dca0

Short description of error
Blender crashes when I click on save in the file browser after pressing ctrl+S twice.

Exact steps for others to reproduce the error

Open default scene.
Press ctrl+S to open the save file dialog.
When it opened, press ctrl+S again.
Click on "Save Blender File".
Crash.

_BLI_assert_abort() (/home/jacques/blender-git/blender/source/blender/blenlib/intern/BLI_assert.c:56)
wm_handler_fileselect_do(bContext * C, ListBase * handlers, wmEventHandler_Op * handler, int val) (/home/jacques/blender-git/blender/source/blender/windowmanager/intern/wm_event_system.c:2316)
wm_handler_fileselect_call(bContext * C, ListBase * handlers, wmEventHandler_Op * handler, const wmEvent * event) (/home/jacques/blender-git/blender/source/blender/windowmanager/intern/wm_event_system.c:2462)
wm_handlers_do_intern(bContext * C, wmEvent * event, ListBase * handlers) (/home/jacques/blender-git/blender/source/blender/windowmanager/intern/wm_event_system.c:2906)
wm_handlers_do(bContext * C, wmEvent * event, ListBase * handlers) (/home/jacques/blender-git/blender/source/blender/windowmanager/intern/wm_event_system.c:2958)
wm_event_do_handlers(bContext * C) (/home/jacques/blender-git/blender/source/blender/windowmanager/intern/wm_event_system.c:3378)
WM_main(bContext * C) (/home/jacques/blender-git/blender/source/blender/windowmanager/intern/wm.c:647)
main(int argc, const char ** argv) (/home/jacques/blender-git/blender/source/creator/creator.c:520)

Added subscriber: @JacquesLucke

#96185 was marked as duplicate of this issue

#89683 was marked as duplicate of this issue

#89075 was marked as duplicate of this issue

Added subscriber: @dfelinto

Changed status from 'Needs Triage' to: 'Confirmed'

I can reproduce this in 2.92 as well as 2.93 and master. It doesn't happen in 2.91.

Added subscriber: @Raimund58

Blender 2.83.15 is also not affected (on Windows)

Added subscriber: @Harley

Actually I just got the exact same behavior in 2.91.0 (for Windows).

Interesting, in Linux I cannot reproduce it in 2.91.

Added subscriber: @Garek

Can confirm on Windows for 2.90.
Working for 2.83.15 as well as 2.83.0

@dfelinto - Interesting, in Linux I cannot reproduce it in 2.91.

Yeah, was doubting myself after seeing other people say otherwise. So I downloaded it again and gave it another try.

Date: 2020-11-25 08:34
Hash: 0f45cab862

And it definitely dies, just as described above. But there are further details that might help:

How quickly you press that second ctrl-S makes a big difference. Best to wait at least a second or two between them just to be sure.

This same issue occurs when pressing the "Cancel" Button on the same form. But NOT when clicking the "X" close in the titlebar. This could be a very interesting clue.

Added subscribers: @coolchou-zhao, @ankitm, @PratikPB2123

Also happens with:

command + O
command + S
Escape/ Enter to save.

Added subscriber: @YKTMU2

Added subscriber: @1D_Inc

Saving action is more than an action - it is a reflex, a part of a CG survival instinct.
A way to save massively learned from the industry standards design solutions has its own memes:

In #88570#1168210, @Harley wrote:
How quickly you press that second ctrl-S makes a big difference. Best to wait at least a second or two between them just to be sure.

Not exactly, it looks like it more depends on if you press

full Ctrl+S several times
or Ctrl+SSSS way.
The second way seems to be stable.

Added subscriber: @michael64

The described crash happens as follows.
In the function GHOST_ScreenToClient the following line is executed:
window->screenToClient(inX, inY, *outX, *outY);
When window is NULL the process crashes but here we are in a code segment that we shouldn't have gotten into in the first place.

Reproducing this bug in a debug build with enabled asserts aborts the process earlier because of a failed assert in wm_event_system.c:

  BLI_assert(ctx_win != win);

So we have ctx_win == win which it shouldn't be but a release build with disabled asserts just marches on and crashes later.

Let's have a look at the big picture.
One could say that most powers users don't like modal dialogs in general. They are overused and annoying, especially when not needed.

As an example, we have a fresh scene with the default cube. Let's move the default cube to the left window border so it's still visible when we press Ctrl-S to save the scene.

Now we have the "Blender File View" save dialog open and the default cube next to the left of it.

Now pressing 'g' to move the default cube does not allow me to move the default cube which seems appropriate to me.
With the save dialog open, I can go to the Object Properties to adjust the x value of the location and click on the number which is in focus but the field does not accept key events, not even moving the cursor.
But I can still click and scrub location.x and see the cube moving while the save dialog is open. Should this even be possible? I don't know, it's a matter of taste I guess.

Likewise I can also press Ctrl-Shift-O to get the recent files dialog while the the save dialog is open. This could even make sense if one just wanted to be reminded of a recent file name before saving.

Back to the details in wm_event_system.c.
A quick workaround like this is not what we are looking for:

        if (BLI_listbase_is_single(&file_area->spacedata)) {
          if (ctx_win == win) {  // This would prevent an immediate
            continue;            // crash but still allows
          }                      // a weird app state.
          BLI_assert(ctx_win != win);

This would still allow a second file window(over the first one to be opened.
Cancelling the second window would remove it as well as the file name, Cancel and Save As buttons. Now the file dialog can only be closed by clicking the 'x'.
My intuition tells me that in this particular case it might be a good idea to not open the second file dialog instance while the first one is still opened.

That's my diagnosis, maybe somebody else knows how to prevent improperly nested file dialogs tastefully.

The described crash happens as follows. In the function GHOST_ScreenToClient the following line is executed: window->screenToClient(inX, inY, *outX, *outY); When window is NULL the process crashes but here we are in a code segment that we shouldn't have gotten into in the first place. Reproducing this bug in a debug build with enabled asserts aborts the process earlier because of a failed assert in wm_event_system.c: ``` BLI_assert(ctx_win != win); ``` So we have ctx_win == win which it shouldn't be but a release build with disabled asserts just marches on and crashes later. Let's have a look at the big picture. One could say that most powers users don't like modal dialogs in general. They are overused and annoying, especially when not needed. As an example, we have a fresh scene with the default cube. Let's move the default cube to the left window border so it's still visible when we press Ctrl-S to save the scene. Now we have the "Blender File View" save dialog open and the default cube next to the left of it. Now pressing 'g' to move the default cube does not allow me to move the default cube which seems appropriate to me. With the save dialog open, I can go to the Object Properties to adjust the x value of the location and click on the number which is in focus but the field does not accept key events, not even moving the cursor. But I can still click and scrub location.x and see the cube moving while the save dialog is open. Should this even be possible? I don't know, it's a matter of taste I guess. Likewise I can also press Ctrl-Shift-O to get the recent files dialog while the the save dialog is open. This could even make sense if one just wanted to be reminded of a recent file name before saving. Back to the details in wm_event_system.c. A quick workaround like this is not what we are looking for: ``` if (BLI_listbase_is_single(&file_area->spacedata)) { if (ctx_win == win) { // This would prevent an immediate continue; // crash but still allows } // a weird app state. BLI_assert(ctx_win != win); ``` This would still allow a second file window(over the first one to be opened. Cancelling the second window would remove it as well as the file name, Cancel and Save As buttons. Now the file dialog can only be closed by clicking the 'x'. My intuition tells me that in this particular case it might be a good idea to not open the second file dialog instance while the first one is still opened. That's my diagnosis, maybe somebody else knows how to prevent improperly nested file dialogs tastefully.

Previously a file dialog in Blender was opened fullscreen instead of a window (which was a nice solution to win 95 window file dialog style, still presented in industry standards - so you can work with long paths and view lots of data).
Does it influence?

This report is not appearing in normal searches and crash wasn't fixed in 3.0.0 release. So setting back #bf_blender project

Added subscriber: @PsychoFisch

Added subscribers: @JulianEisel, @lichtwerk

Caused by 796412dca0 btw.

@JulianEisel:I know you have a fix ready already, mind getting that reviewed/pushed somehow?

Added subscriber: @kmcurry

Added subscriber: @Jake-Faulkner

Alright so:

D13441: Fix #88570: Crash when saving after pressing ctrl+S twice.: This solves the crash reported here. It tries to do so on the design level by introducing a well defined context that "owns" the file browser and for executing the file operation in. It may unveil some further issues.
D14905: Fix for crash opening the file selector multiple times (by @ideasman42): This is one such issue unveiled by the fix above. It fixes dangling pointers.
D13459: Restrict temp/Maximized/Fullscreen window editing (by @Jake-Faulkner): This prevents more undefined state that causes issues. Editing of temporary/modal screens is supposed to be disabled, see 87aca8bd02, but that commit didn't cover enough cases.

Added subscriber: @ideasman42

This issue was referenced by bc8e030a84

This issue was referenced by 7849b56c3c

Changed status from 'Confirmed' to: 'Resolved'

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Regression: Crash when saving after pressing ctrl+S twice. #88570