Thunar tree pane becomes out of sync and uses 100% CPU

changed the description

Ok, now I can confirm. Partial repo:

Do some operations outside of Thunar that remove/add directories without Thunar noticing
Press F5 to see the missing directory again in Thunar
Enter the previous missing directory and navigate to a subfolder within it

-> Tree pane out of sync, CPU usage at 100%

Meh, again this one :/ Thank you for reporting !

Think I can make the reproducer more concrete:

Open some folder in thunar, switch to tree-view
expand the folder in tree-view, so that direct sub folders are visible
Open a terminal in the folder: mkdir -p foo/bar
enter foo/bar in thunar

Patches would be very welcome !

@lastonestanding any me once discussed that it probably would make sense to delay the tree-walking loop thunar_tree_view_cursor_idle in some way, so that it at least does not spike the CPU to 100%, but just emits some warnings instead.

We as well though about limiting the search depth .. though I dont remember what was the result of that discussion.

Since I fear there always will be some border-case leading to infinite tree-walk, I think it would be good to go for such protection mechanics.

I think we agreed that throttling the function would help.

However, for this bug we have to look at the following code part:
https://gitlab.xfce.org/xfce/thunar/-/blob/master/thunar/thunar-tree-model.c#L1288

The function thunar_file_is_directory may return FALSE for the folder "foo".

I cannot reproduce this in either master or 4.15.1 using alexxcon's method (or haarp's). How are you entering the new directory? I've tried clicking on the icon, navigating via the tree view, and entering the path directly in the location bar, but without seeing this bug. Are there any more specific things I can do to see it?

So, for you the new directory always appears in the tree view? I did some test runs and it was often missing.

and it was often missing.

This is also my experience.

Ok, so after trying this a lot more, I did manage to get part of this bug on 4.15.1. With the side pane in tree-mode and the current directory expanded in the side pane, I repeatedly did mkdir -p dir*/sub with * being 1,2,3,4,..., and a few variations of this (eg dir_*/sub, dir*/subdir, etc). I got a few cases where the new parent directory (ie dir*) appeared in the side pane without a collapse/expand arrow, and when I clicked into it the sub directory was not shown, but appeared when I pressed F5. I did not see the side pane being out of sync or any abnormal CPU usage. I had to run the command many tens of times to see the bug happen once, though.

I think the tree pane and main view need to diverge by more than one dir level before the tree loop starts.

@haarp so for you, do you (at least sometimes) see the bug if you eg

do mkdir -p sub1/sub2 and thunar shows sub1 but not sub2
enter sub1 in thunar
press F5 to make sub2 appear
enter sub2 in thunar

because that is what I've done a few times, but the tree pane and main view stay in sync and there is no extra CPU usage on my system. Am I missing something? Thanks!

It's a bit tricky to get Thunar confused enough so it doesn't see the dirs. This has worked for me just now:

mkdir -p /tmp/sub1/sub2/sub3
Enter sub3 in Thunar
rm -r /tmp/sub1 -> Thunar jumps back to /tmp
mkdir -p /tmp/sub1/sub2/sub3
In the previous Thunar window, enter sub1 via the main view -> The tree view didn't follow.
In the same window, enter sub2 via the main view -> CPU usage becomes high (and sub3 is shown as an unknown-type file).

So in addition to the tree pane CPU bug, there is a bug that confuses Thunar as to which directories exist, but which is an useful way to trigger the tree pane bug.

Thanks, @haarp! So using these steps I can reproduce this bug consistently with both 4.15.1 and the current master, including the out-of-sync side pane and high CPU usage.

Does the patch for #355 (closed) eliminate the trigger for this bug (missing entry in tree view)?

@lastonestanding Yes, issue355.diff in #355 (closed) does eliminate this trigger. Tho I suspect there are additional triggers.

(sorry for the delay, it seems I wasn't receiving some mail from this Gitlab instance)

mentioned in issue #355 (closed)

mentioned in issue #402 (closed)

Tho I suspect there are additional triggers.

Yup, turns out this was correct, see #402 (closed).

We could stop the loop early on if the toplevel path is not present in the tree view.

@alexxcons Should we not be returning FALSE in thunar_tree_view_cursor_idle?

issue351.diff

After reading the doc, I suppose stopping the loop makes perfect sense for if (G_UNLIKELY (view->current_directory == NULL)).

I suppose I confused the return values after switching to gboolean done = FALSE; above during the latest patches.

For if (view->select_path != NULL) I first I thought it should be ok to try opening the tree after force-selecting a certain path. Though using FALSE prevents cursor jumping on unmount, which imo is nice, so I suppose FALSE it the right thing to do. (Thought we even have an issue for that, but did not find any)

Dont select anything instead of hopeing to catch some possibly newly created root node looks very reasonable to me ... better safe than sorry.

To sum up: Looks good to me, I would push it, if you are done with it.

Unrelated: The return value done is still confusing. !done is returned at the end .. so if we are done, aka done = TRUE, "not done" is returned :P (I'll rename it to 'full_path_found' after this fix got pushed and add some documentation)

I have not tested it properly yet (and we know that it is very easy to introduce bugs in this part of the code).

Ok, just let me know when you think it is ready ! I'll meanwhile install it for my daily use and report here if I see something suspicious.

Status report? :)

Did not face any downsides. Though after 2 weeks or so I overwrote it to test something else and ofc forgot to go back ;)

If you are fine with it, I would push it to master .. that would give us a broader testing audience.

Please go ahead.

Suggestion for commit message:
Unset tree view cursor if toplevel path cannot be determined

Regarding the "heavy" looping, would switching to g_timeout_add_(seconds_)full be a solution? Maybe use a counter on top of that to terminate after a certain count.

Yes, I guess so. I would go for g_timeout_add_full, since I suppose a second is too much to wait for tree-buildup in case of access problems/slow access. How about 100ms ?

I guess after ~5 seconds loading failure it should be ok to terminate the loop by the counter

added 1. Performance 5. Side Pane - Tree labels

I guess after ~5 seconds loading failure it should be ok to terminate the loop by the counter

This is probably the best solution. So, we need a timer and compare against it?

Did not think about the details so far. I suppose sufficient to get a timestamp in the beginning and substract it from a "now" stamp at some point in the loop.

Not sure if there are more elegant options.

changed milestone to %Xfce 4.18

mentioned in commit 545d70a1

mentioned in issue #510 (closed)

Is this problem solved? I cannot reproduce this behavior with current master.

Yes and no. Indeed it looks like @lastonestanding's patch fixed the last culpits.

Though thunar_tree_view_cursor_idle is still rather complex, and I suppose there is still the risk in never finding the full path in some corner-cases, leading to infinite calls of thunar_tree_view_cursor_idle and 100% CPU.

So I think it still would be worth it to use g_timeout_add_full or some other mechanism in order to stop tree-loading after a while.

@MShrimp4 : Yet another one reported which smells like it would be related: #510 (comment 34438)

Thunar tree pane becomes out of sync and uses 100% CPU

Designs

Child items 0

Activity