Fix #112599: Blender freezes for half a minute when opening File Browser #116692

Germano Cavalcante · 2024-01-01T17:00:06+01:00

Germano Cavalcante commented

2024-01-01 17:00:06 +01:00

The delay is caused by Shortcut->Resolve(0, SLR_NO_UI | SLR_UPDATE)
to locate potentially moved or renamed paths.

The issue was resolved by adding the SLR_NOSEARCH flag..

When investigating the issue, it was not difficult to find where the delay occurred.

The hardest part was understanding why Blender calls Shortcut->Resolve.

The documentation of this method shows that it can behave differently depending on the flags that are used , and the one involved in the slowness is SLR_UPDATE.

Resolving/refinding the directory of a 'Shell link' doesn't seem to be something within the scope of the File Browser. The only advantage of doing this is that BLI_is_dir(entry->redirection_path) may return true after the directory is resolved and thus displayed in the File Browser if filter is enabled.

But the adverse case seems to be much worse, as in addition to #112599, this action of resolving 'Shell link' could be the cause of the long wait to open Blender reported in #116675, #115935, #115806, #114247, #115008

So Blender better not try to fix these 'Shell link's.

The delay is caused by `Shortcut->Resolve(0, SLR_NO_UI | SLR_UPDATE)` to locate potentially moved or renamed paths. The issue was resolved by adding the `SLR_NOSEARCH` flag.. --- When investigating the issue, it was not difficult to find where the delay occurred. The hardest part was understanding why Blender calls `Shortcut->Resolve`. The [documentation](https://learn.microsoft.com/en-us/windows/win32/api/shobjidl_core/nf-shobjidl_core-ishelllinkw-resolve) of this method shows that it can behave differently depending on the flags that are used , and the one involved in the slowness is `SLR_UPDATE`. Resolving/refinding the directory of a 'Shell link' doesn't seem to be something within the scope of the File Browser. The only advantage of doing this is that `BLI_is_dir(entry->redirection_path)` may return `true` after the directory is resolved and thus displayed in the File Browser if filter is enabled. But the adverse case seems to be much worse, as in addition to #112599, this action of resolving 'Shell link' could be the cause of the long wait to open Blender reported in #116675, #115935, #115806, #114247, #115008 So Blender better not try to fix these 'Shell link's.

Germano Cavalcante added the

Platform

Windows

Module

Platforms, Builds & Tests

labels 2024-01-01 17:00:06 +01:00

Germano Cavalcante added 1 commit 2024-01-01 17:00:14 +01:00

66c6dbbff3 Fix #112599 : Blender freezes for half a minute when opening File Browser

Considering the disadvantage of waiting for
`Shortcut->Resolve(0, SLR_NO_UI | SLR_UPDATE)` to find the path that
may have been moved or renamed, it seems better to just not call this
function.

A disadvantage of this change is that, if the target directory is not
found (because it was moved or renamed), Blender's File Browser will
not access the shortcut nor display it if it is filtered.

One advantage is that the user will not have to mysteriously wait
minutes for the inaccessible directory of a shortcut to be found.

Germano Cavalcante requested review from Harley Acheson 2024-01-01 17:01:05 +01:00

Germano Cavalcante requested review from Ray molenkamp 2024-01-01 17:01:06 +01:00

Harley Acheson commented

2024-01-01 20:47:42 +01:00

Wow, great work tracking this down.

I would have never guessed that Shortcut->Resolve was optional there. And I would have assumed that problems with that would have come to light when we first added support for shortcuts, or when people started putting them in offline storage, etc. So wonderful that you looked here.

When testing for this complaint, is the long time waiting for this from its initial check of "does this exist?" or from its subsequent search for it?

This function seems to use just two methods of finding an item if not found initially. The first being the distributed link tracking service, and the second being a search of the last known directory, subdirectories, etc. Both of these can be turned off with flags, so wondering if the problems go away if we turn off both methods by adding SLR_NOTRACK and SLR_NOSEARCH to the flag. If not, then best to not call it as you do in this PR.

But if that does improve things we might be able to get away with calling it with just SLR_NO_UI | SLR_UPDATE | SLR_NOSEARCH, so allowing distributed link tracking but not the recursive search. We could even add a custom timeout to this function, by setting the high word of the Flags argument to a timeout in milliseconds (only works when SLR_NO_UI is also set).

Wow, great work tracking this down. I would have never guessed that `Shortcut->Resolve` was _optional_ there. And I would have assumed that problems with that would have come to light when we first added support for shortcuts, or when people started putting them in offline storage, etc. So wonderful that you looked here. When testing for this complaint, is the long time waiting for this from its initial check of "does this exist?" or from its subsequent search for it? This function seems to use just two methods of finding an item if not found initially. The first being the distributed link tracking service, and the second being a search of the last known directory, subdirectories, etc. Both of these can be turned off with flags, so wondering if the problems go away if we turn off both methods by adding SLR_NOTRACK and SLR_NOSEARCH to the flag. If not, then best to not call it as you do in this PR. But if that does improve things we might be able to get away with calling it with just `SLR_NO_UI | SLR_UPDATE | SLR_NOSEARCH`, so allowing distributed link tracking but not the recursive search. We could even add a custom timeout to this function, by setting the high word of the Flags argument to a timeout in milliseconds (only works when SLR_NO_UI is also set).

Pratik Borhade referenced this pull request

2024-01-02 07:04:56 +01:00

Blender hangs for multiple minutes when opening blend file from networked drive #116675

Timothy Cook commented

2024-01-03 06:07:56 +01:00

First-time contributor

Oh man I'm glad I found this, and I want to confirm this bug. I just recently started networking a shared file folder between my Mac and PC that I keep on both desktops, and whenever I tried to view the Desktop through Blender's File Browser Blender would freeze... well I though it was freezing and locking up.

Quick question. Is there a way that this could get pushed to 3.6?

Thanks for this.

Oh man I'm glad I found this, and I want to confirm this bug. I just recently started networking a shared file folder between my Mac and PC that I keep on both desktops, and whenever I tried to view the Desktop through Blender's File Browser Blender would freeze... well I though it was freezing and locking up. Quick question. Is there a way that this could get pushed to 3.6? Thanks for this.

Germano Cavalcante added 3 commits 2024-01-03 15:31:15 +01:00

8f87074273 Revert "Fix #112599 : Blender freezes for half a minute when opening File Browser"

This reverts commit 66c6dbbff3.

913abb3dba Call `IShellLinkW::Resolve` with `SLR_NOSEARCH`

cdde784309 Merge remote-tracking branch 'origin/main' into fix_112599

Germano Cavalcante commented

2024-01-03 15:34:49 +01:00

SLR_NOSEARCH solves the problem.
Using this flag is indeed an advantage, but it may not be appropriate to call Shortcut->Resolve when the File Browser is simply navigating through a directory with Shell Links.

In Windows, the Desktop directory is usually filled with shortcuts to applications. Thus, simply navigating the Desktop, Blender calls Shortcut->Resolve for each of these shortcuts.

Ideally, in my opinion, calling Shortcut->Resolve should occur only when accessing the Shell Link.

To determine if a Shell Link points to a directory (even with the target directory broken and without Shortcut->Resolve), we could do something like this:

bool is_pointing_to_dir = false;
WIN32_FIND_DATA wfd;
hr = Shortcut->GetPath(target_utf16, FILE_MAXDIR, &wfd, 0);
if (SUCCEEDED(hr)) {
  is_pointing_to_dir = (wfd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY);
}

This way we could display the shortcut in the File Browser even though it is broken.

`SLR_NOSEARCH` solves the problem. Using this flag is indeed an advantage, but it may not be appropriate to call `Shortcut->Resolve` when the File Browser is simply navigating through a directory with `Shell Links`. In Windows, the Desktop directory is usually filled with shortcuts to applications. Thus, simply navigating the Desktop, Blender calls `Shortcut->Resolve` for each of these shortcuts. Ideally, in my opinion, calling `Shortcut->Resolve` should occur only when accessing the `Shell Link`. To determine if a `Shell Link` points to a directory (even with the target directory broken and without `Shortcut->Resolve`), we could do something like this: ```cpp bool is_pointing_to_dir = false; WIN32_FIND_DATA wfd; hr = Shortcut->GetPath(target_utf16, FILE_MAXDIR, &wfd, 0); if (SUCCEEDED(hr)) { is_pointing_to_dir = (wfd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY); } ``` This way we could display the shortcut in the File Browser even though it is broken.

broken_shortcut.lnk

2.1 KiB

Germano Cavalcante commented

2024-01-03 16:01:50 +01:00

Quick question. Is there a way that this could get pushed to 3.6?

As long as the fix is compatible, and 3.6 is within the LTS period. This fix appears to qualify for backport:
https://projects.blender.org/milestones

> Quick question. Is there a way that this could get pushed to 3.6? As long as the fix is compatible, and 3.6 is within the LTS period. This fix appears to qualify for backport: https://projects.blender.org/milestones

Harley Acheson approved these changes 2024-01-03 17:28:22 +01:00

Harley Acheson commented

2024-01-03 17:52:47 +01:00

SLR_NOSEARCH solves the problem.

Awesome! Yes, this change should go into main and be added to the backport lists.

it may not be appropriate to call...

Maybe? Now that we know that resolve is optional, then it should do almost nothing if the link is to a valid file, so should be super fast. I'm hoping so anyway.

calling Shortcut->Resolve should occur only when accessing the Shell Link...

That could work. We could add your earlier "fix if broken" argument, and skip that in the only place it is called now, when processing folder contents. Right now If that call is unsuccessful we make it look like a hidden file by giving it our FILE_ATTR_HIDDEN attribute. But since we are not fixing them we could instead add a new flag that indicates error or inaccessibility, FILE_ATTR_ERROR maybe. That could also be added on failure inside of BLI_file_attributes. I could color it red or add a stopsign icon or something to it. Then later we could choose to call BLI_file_alias_target with that argument true for items that are FILE_ATTR_ALIAS and FILE_ATTR_ERROR`. If we actually want to.

This feature is mostly for shortcuts to blend files. Are we really going to get people complaining that the shortcut looks broken after they move the target file but Blender should automatically fix it for them?

> SLR_NOSEARCH solves the problem. Awesome! Yes, this change should go into main and be added to the backport lists. > it may not be appropriate to call... Maybe? Now that we know that resolve is optional, then it should do almost nothing if the link is to a valid file, so should be super fast. I'm hoping so anyway. > calling Shortcut->Resolve should occur only when accessing the Shell Link... That could work. We could add your earlier "fix if broken" argument, and skip that in the only place it is called now, when processing folder contents. Right now If that call is unsuccessful we make it look like a hidden file by giving it our FILE_ATTR_HIDDEN attribute. But since we are not fixing them we could instead add a new flag that indicates error or inaccessibility, FILE_ATTR_ERROR maybe. That could also be added on failure inside of `BLI_file_attributes`. I could color it red or add a stopsign icon or something to it. Then later we could choose to call `BLI_file_alias_target` with that argument true for items that are `FILE_ATTR_ALIAS` and FILE_ATTR_ERROR`. If we actually want to. This feature is mostly for shortcuts to blend files. Are we really going to get people complaining that the shortcut looks broken after they move the target file but Blender should automatically fix it for them?

Germano Cavalcante added 1 commit 2024-01-03 18:19:05 +01:00

e0ba9cccec Merge branch 'main' into fix_112599

Germano Cavalcante merged commit 81017772f5 into main

2024-01-03 18:26:34 +01:00

Germano Cavalcante deleted branch fix_112599

2024-01-03 18:26:36 +01:00

Germano Cavalcante referenced this issue from a commit

2024-01-03 18:26:39 +01:00

Fix #112599: Blender freezes for half a minute when opening File Browser

Germano Cavalcante referenced this issue from a commit

2024-01-08 14:42:48 +01:00

Tools: Add 'This week' and 'Next week' fields to weekly report generator