Very serious - Folders being displayed as empty after being renamed

[rafcon-dev]

Hi.
So, I've found a pretty serious bug that could easily lead to some data loss, as it'd led me had I not backups.

How ot reproduce:
Have two folders:
folderA - has files inside
folderB - is empty

delete folderB
rename folderA to folderB
now folderB (previsouyl folderA) will appear empty, even though it's not!

I'm guessing this is due to some sort of caching, but it's quite dangerous. Shouldn't the folders refresh every second or something like that?

Edit: Indeed, I just tested lf and dolphin opened side by side. If I create a file in lf, it shows up in dolphin after a second. If I do the same in dolphin, it doesn't show up in lf! This unfortunately makes lf unusable, tbh. If you need help in fixing this one, I can give a hand.

Edit2: I just tested with ranger, and it indeed reacts to a new created file in dolphin, as it should. Midnight Commander doesn't, but if you can refresh the folder with CTRL+R (which sucks btw)

[lahwaacz]

Read about the period option in the documentation.

[lahwaacz]

If you do the "How to reproduce" steps in lf instead of an external file manager, it behaves correctly – i.e. the folderA does not appear empty. So there is no bug.

[rafcon-dev]

Indeed, the period option should give the expected behaviour. It is very weird that it is not defaulted to 1, I noticed someone just a few days ago with the same issue, this is clearly a usability problem that goes against what's usually expected of a file manager. I've noticed that dolphin does the same thing at times, which also sucks.

Regarding the bug, that's a classic "It works for me!" mate. Here's a gif so I can more clearly explain myself.

What's worse is that not even the period option at 1 does anything to solve this.

[lahwaacz]

The motivation for zero period by default is explained in the FAQ: "Features that have a chance to cause performance slowdown are turned off by default."

Can you show your lfrc?

[rafcon-dev]

hmm, I mean I'm all for performance, but the main goal of a file manager is to display the files that currently exist in a folder. If it fails at that for whatever reason, it's unfortunately failing it's main goal. It's 2022, we have crappy fully responsive apps written in freaking JS but we can't have a responsive file manager in the terminal? It don't think such a basic thing would be considered "performance bloat".
I get it might lag on remote folders, but couldn't it be asynced or something?

As for the lfrc, here it is, nothing special really.

set ignorecase true

set icons true
set shell zsh


set shellopts '-eu'


set ifs "\n"


set previewer ~/.config/lf/preview
set cleaner ~/.config/lf/cleaner


cmd fzf_jump ${{
    res="$(find . | fzf --reverse --header='Jump to location' | sed 's/\\/\\\\/g;s/"/\\"/g')"
    if [ -d "$res" ] ; then
        cmd="cd"
    elif [ -f "$res" ] ; then
        cmd="select"
    else
        exit 0
    fi
    lf -remote "send $id $cmd \"$res\""
}}
map <c-t> :fzf_jump

cmd z %{{
    result="$(zoxide query -- "$1")"
    lf -remote "send ${id} cd '${result}'"
}}

cmd zi ${{
    result="$(zoxide query -i -- "$1")"
    lf -remote "send ${id} cd '${result}'"
}}

map Dd delete

cmd mkdir %{{
	printf "Directory Name: "
	read ans
	mkdir $ans
    	lf -remote "send ${id} cd '${ans}'"
}}

map R reload
set period 1

map c clear
map Md mkdir
map gd cd ~/Downloads

map n jump-prev
map m jump-next

[lahwaacz]

Hmm, now for me the caching issue sometimes happens and sometimes it doesn't...

[rafcon-dev]

"Good" to know. There's definitely something devious going on under the surface mate!

[DusanLesan]

Can you try chaining commands for this action?
Something like this:

map r :rename; reload

[dase78]

I had the same issues, also if I created new directories or files in them via a self built command in lfrc.

DusanLesans comment helped out, after chaining my commands with reload evereything works as expected.

(set period ... did not help).

EDIT: no, does not help always, happens randomly.

[joelim-work]

This is caused by the dircache feature, which caches dir data structures by its path:

lf/nav.go

Lines 419 to 430 in b47cf6d

	if gOpts.dircache {
	d, ok := nav.dirCache[path]
	if !ok {
	d = nav.loadDirInternal(path)
	nav.dirCache[path] = d
	return d
	}
	nav.checkDir(d)
	return d
	}

The problem is that when deleting a directory, the corresponding entry is not deleted from the cache. This is the code that performs the delete:

lf/nav.go

Line 1392 in b47cf6d

if err := os.RemoveAll(path); err != nil {

This means that when another directory is renamed to the path of the deleted directory, it will load the stale entry from the cache, causing it to be displayed incorrectly.

Setting the period option won't help either, since it only checks if the contents of a directory have changed, not if the directory itself has moved to a new location.

It looks like deleting the stale entry immediately doesn't work, since lf thinks the deleted directory is still the current file and tries to reload it again. I think it would be easier to just ensure there won't be a stale entry immediately after a rename action:

--- a/nav.go
+++ b/nav.go
@@ -1426,6 +1426,7 @@ func (nav *nav) rename() error {
                return err
        }

+       delete(nav.dirCache, newPath)
        dir := nav.loadDir(filepath.Dir(newPath))

        if dir.loading {

---

In the meantime, you can also disable directory caching by adding set nodircache to your config file.

[dase78]

In the meantime, you can also disable directory caching by adding set nodircache to your config file.

No, please don't do this!

If you set "nodircache" lf does not remember what you selected the last time and deletes (if you delete) where you are on at the moment. I had this several times now and deleted things I did not want to delete. It seems like a mixture of bug and sluttery (by myself) to me.

[joelim-work]

If you set "nodircache" lf does not remember what you selected the last time and deletes (if you delete) where you are on at the moment. I had this several times now and deleted things I did not want to delete. It seems like a mixture of bug and sluttery (by myself) to me.

@dase78 Do you have a minimal set of steps to reproduce this issue? I don't actually use set nodircache myself, so I'm not aware of any issues that it might cause.

[dase78]

@joelim-work Yes, I reproduced it now again:
set nodircache in ~/.config/lf/lfrc
Go to any directory and delete any file which is not in first line in directory (with the builtin delete command). It will delete the first file even if you are asked to confirm the chosen file.

Could you also try, this seems serious?

[joelim-work]

@dase78 I tried it just now with a config file containing nothing but set nodircache, using a few files named from a to e. But I still couldn't reproduce your issue. Maybe someone else reading this can try for themselves?

2023-02-10.22-44-26.mp4

[joelim-work]

BTW, this is the code that performs the actual delete. The first thing it does is call nav.currFileOrSelections, which returns a list of selected files, or the file at the current position if there aren't any selected files.

lf/nav.go

Lines 1377 to 1397 in b47cf6d

	func (nav *nav) del(app *app) error {
	list, err := nav.currFileOrSelections()
	if err != nil {
	return err
	}
	go func() {
	echo := &callExpr{"echoerr", []string{""}, 1}
	errCount := 0
	nav.deleteTotalChan <- len(list)
	for _, path := range list {
	nav.deleteCountChan <- 1
	if err := os.RemoveAll(path); err != nil {
	errCount++
	echo.args[0] = fmt.Sprintf("[%d] %s", errCount, err)
	app.ui.exprChan <- echo
	}
	}

If you want to get to the bottom of this, you can try adding some log.Printf statements into the code to help debug the issue.

[joelim-work]

Oh, and also I am starting to think that this is a separate problem and a new issue should be created, rather than discussing it here.

[dase78]

So I tried again several things, I think I know, what was happening:

I could reproduce my failure but I guess it was, because I had set: map <delete> :delete; reload, so when I pushed , it asked for confirmation for the right file but then deleted the new "reloaded" one. I hope, this makes sense, how I explained.

I took out the "reload" and now everything works as expected with deleting files.

[I don't know, should we report this?] Can you reproduce?

[joelim-work]

I could reproduce my failure but I guess it was, because I had set: map :delete; reload, so when I pushed , it asked for confirmation for the right file but then deleted the new "reloaded" one. I hope, this makes sense, how I explained.

@dase78 I tried this just now and can confirm that is the problem you're experiencing.

The reload command clears the directory cache (nav.dirCache) and reloads any directories, but also stores the file at the current position (in nav.dirs[len(nav.dirs)-1].files) so that it can be restored afer the reload:

lf/nav.go

Lines 568 to 581 in b47cf6d

	nav.dirCache = make(map[string]*dir)
	nav.regCache = make(map[string]*reg)
	wd, err := os.Getwd()
	if err != nil {
	return fmt.Errorf("getting current directory: %s", err)
	}
	curr, err := nav.currFile()
	nav.getDirs(wd)
	if err == nil {
	last := nav.dirs[len(nav.dirs)-1]
	last.files = append(last.files, curr)
	}

When a directory is reloaded, the current file is initially reset to the first entry. However the original current file prior to the reload will be restored (via the sel function) if the directory cache is enabled:

lf/app.go

Lines 391 to 403 in b47cf6d

	case d := <-app.nav.dirChan:
	app.nav.checkDir(d)
	if gOpts.dircache {
	prev, ok := app.nav.dirCache[d.path]
	if ok {
	d.ind = prev.ind
	d.sel(prev.name(), app.nav.height)
	}
	app.nav.dirCache[d.path] = d
	}

This is why the current file after a reload remains unchanged if directory caching is enabled, and reset to the first entry if directory caching is disabled. I believe this is by design though, so I don't think there's any issue there.

Adding map <delete> :delete; reload will also solve the original issue, because reload clears the directory cache, so there is no stale entry when the directory is renamed. But I think it would be a lot better to actually fix this bug inside the application itself, rather than forcing users to have this kind of workaround in their config file.

[joelim-work]

I think it's also worth pointing out that this issue also occurs when you rename the original directory to something else instead of deleting it.

The original example given was:

folderA - has files inside
folderB - is empty

delete folderB
rename folderA to folderB

If you change the step delete folderB to rename folderB to folderC, this issue will also occur, despite the fact that nothing is actually being deleted.

[dase78]

But I think it would be a lot better to actually fix this bug inside the application itself, rather than forcing users to have this kind of workaround in their config file.

Yes, I agree.

And chaining reload to my several commands (deleting, renaming, creating new dir/file, pasting) did not always solve the original issue, but I could not always reproduce it - not updating the info happened qutite randomly. But setting nodircache seems better at the moment.