I have a feeling this is close, but not quite...

The problem here isn't the resuse of temporary windows, but that a second call to WM_window_open is even made.

I think you can take your changes here, replace the win = win->parent; with return; then change the comment to something like "Do not open new File Browser if one is already open."

Having said that it is very possible that your code could further be reduced to:

/* Do not open new File Browser if one is already open. */
if (CTX_wm_space_file(C)) {
  return;
}

I don't think there are any situations where this would not work well, but worth checking.

I think you can take your changes here, replace the win = win->parent; with return; then change the comment to something like "Do not open new File Browser if one is already open."

The difference would be that if a fileselector was open and you chose a different operator that uses a file selector, say Alt+S for image save followed by Ctrl+Alt+S, returning immediately would mean the fileselctor wouldn't update from the image save to the mainfile save dialog, effectively disabling other operators that use a fileselector until the existing one is closed.

Having said that it is very possible that your code could further be reduced to:
I don't think there are any situations where this would not work well, but worth checking.

The only way I think this could cause an issue is in cases where a SPACE_FILE is not the only space in the window, and I don't see anywhere that could happen... updated, thanks!

I'd add a check that win->parent is not null but otherwise looks good to me.

/* If this is a File Browser window, handlers go in the parent. */
if (CTX_wm_space_file(C) && win->parent) {
  win = win->parent;
}

Noticed that CTX_wm_space_file() returns NULL if the space isn't currently under the cursor, meaning that it doesn't guard from the crash reliably. The first solution doesn't have this issue, so reverting to that.

This is still letting that second WM_window_open proceed. So do the following:

• Edit / Preferences / File Paths
• Click on a folder icon to change any path
• Hit Ctrl-S while that window is open, see that "Accept" button changes to "Save Blender File".

Doing an early exit (my first comment) fixes this but then breaks other things.

The fix has to address both issues otherwise I'd rather just tell people "don't hit ctrl-s twice"

Still behaves oddly, as described in my earlier comment about "Edit / Preferences / File Paths"

I can't see that this "wm_window.c" change is doing anything for you here. When asked to create a new window we are looking at the existing ones in case we don't want to make a second copy of one that isn't meant to have multiples. The existing logic is "don't make a new window if there is one that is temp, and has just a single editor matching the one being asked for". I can't see how relaxing the "has just a single editor" helps with the issue at hand, unless I am missing something.

To be honest, I don't think the solution to this problem will be related to File Browser window creation at all. You might need to back up one layer or so...

Using current master, open an EXISTING file, so one that already has a file name. Hit ctrl-S and see that it just saves without a bringing up a dialog. Now select "File / Save As" to bring up the File Browser. While that is up, hit Ctrl-S and see that the file is saved, properly, and without causing any kind of error, even though the "Save As" File Browser is already open.

The reported error occurs when the file has not been previously saved. This is because "Save" and "Save as" are different operators (save_mainfile and save_as_mainfile) that share some functions between them. If you look at wm_save_mainfile_invoke you will see where we check if the file has a name (G.save_over) and either just immediately saves, or calls WM_event_add_fileselect. You could probably just put your checks there to see if there is already a File Save happening. checking there won't affect other uses of the File Browser since this only for saving the main file.

So my gut feeling is that something as simple as this might be enough:

@Jake (Welp) - Did a few quick tests with my idea of just checking for CTX_wm_space_file inside wm_save_mainfile_invoke and not finding any problems even in all the odd circumstances I could think of. I am assuming though that you've gotten quite good at testing this now, so maybe you will find a problem situation I am missing or you can confirm it is working for you.

In D12592#333569, @Campbell Barton (campbellbarton) wrote:

Requesting changes as the crash still happens with this patch.

Could you list the steps to cause the crash?

As pointed out my suggested patch above would not detect a running File Browser in a window when your mouse is over a different window. The following should check for that but also if it is open as a maximized temporary area in the main window.

In D12592#334238, @Harley Acheson (harley) wrote:

As pointed out my suggested patch above would not detect a running File Browser in a window when your mouse is over a different window. The following should check for that but also if it is open as a maximized temporary area in the main window.

FileCrash2.diff7 KBDownload

If you have an open File Browser area, and place the cursor within the area, Ctrl+S will not function. The crash is also still present with other operators such as open_mainfile and recover_auto_save.

@Jake (Welp) - If you have an open File Browser area, and place the cursor within the area, Ctrl+S will not function.

Yes, that is just caught by that if (CTX_wm_space_file(C)) test. Since this issue does not seem to happen when opening the file browser as a maximized area you could just take that out...

Just trying to offer you suggestions.

@Jake (Welp) - The crash is also still present with other operators such as open_mainfile and recover_auto_save.

Here is using that check as a poll function and using it with those other operators:

Changing the file operation while browsing is a feature, not a bug. So you can for example do File → Open, decide that you want to save the file first and do that using Ctrl+S with the File Browser already open. Or say you decide to make a backup while browsing for a media file. There is plenty of code to make sure that works fine. So this shouldn't be fixed via the poll functions. The File Browser handler code should handle this case properly and replace the file operation.

Looking into this today, I decided to do some further reaching changes to really get to the root of the issue. Submitted D13441: Fix T88570: Crash when saving after pressing ctrl+S twice.. This attempts to properly define rules for how the parent window of a file operation is selected and how we deal with context in return.

We can keep this patch open, since it's not said yet that my patch doesn't cause more problems. First off I need some feedback on it from other devs, to see if they agree with my design. My patch also shouldn't be included in 3.0, it's too risky.