Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dolphin does not pick up if new files have been added to smb folders, even if clicking on it again. only a hard refresh will show them #221

Open
star-buck opened this issue Jan 3, 2020 · 11 comments
Assignees

Comments

@star-buck
Copy link
Contributor

star-buck commented Jan 3, 2020

this is from a fresh install, not sure what we can check if it run out of something or such

see maybe also #71

@star-buck star-buck changed the title dolphin does not pick up if files have been added to smb folders, even if clicking on it again. only a hard refresh will show them dolphin does not pick up if new files have been added to smb folders, even if clicking on it again. only a hard refresh will show them Jan 3, 2020
@davidedmundson
Copy link

From what I can tell reading KIO samba code there is no code to watch for changes over samba, so the fact that auto-refresh doesn't work is not too surprising.
Apparently there are some mechanism in samba available, @apachelogger is probably our resident samba expert.

But we should still reload when you reopen the folder - KIO does do some explicit caching, which kinda makes sense to keep things fast, but is clearly too aggressive and something we can fix. I shall take a look at purely the cache side.

FWIW: A quick test of nautilius + gvfs also failed to update automatically on samba changes, but does reload when you reopen the directory.

@hsitter
Copy link
Member

hsitter commented Jan 7, 2020

I think this is actually tricky. KIO slaves are entirely passive, they have no way to send signals (such as "something changed") nor do they have lifetime control over themselves. So, I think the way this works for the few slaves that actually do this is

  • via kdirwatch which I presume is only used for file:// URIs and relies on inotify/fam, so it needs an underlying mounted filesystem
  • or via a bespoke kded module so as to bypass lifetime control and push notifications into kio from the outside

The latter could technically work for smb. I expect there's plenty of challenges though, since there seems to be no complex slave using that system.

@star-buck star-buck transferred this issue from blue-systems/netrunner-next Feb 23, 2020
@star-buck star-buck pinned this issue Feb 23, 2020
@hsitter
Copy link
Member

hsitter commented Feb 25, 2020

We'll have to hunt down an upstream dev I think.

smbc_notify is existing API to get notification on an open directory. It is working as expected in 4.12 but seeing as libsmbclient has no event loop calling this function is blocking until you cancel the notification from the callback you get from smbc_notify (in the same thread). Trouble is that somehow all of this doesn't seem particularly thread safe. No matter how I try to bend it, it will crash once threading is involved, heck even trying to construct a standalone context in a thread blows up for some reason. So that sucks and only leaves us with a multi-process monitoring solution which would be suuuuper inefficient. Alternatively we could choose to only monitor one path for changes. It's all very awkward as it looks right now.

@star-buck
Copy link
Contributor Author

star-buck commented Feb 25, 2020 via email

@Feverfew
Copy link
Member

Feverfew commented Feb 26, 2020

I can take a look. Note that KIOFuse has the same issue. We've gone for refreshing the directory every 30 seconds. It's currently in review https://gitlab.com/Vogtinator/kio-fuse/merge_requests/10

i imagine it will be easier to just refresh the directory every 30 seconds than implementing smbc_notify. I'll take a look and get back.

For future reference: https://gitlab.gnome.org/GNOME/gvfs/issues/156

@hsitter
Copy link
Member

hsitter commented Feb 26, 2020

Nice, the gnome bug actually has a reference to the samba bug about context not being thread safe.

Which means auto refresh is the most easily achievable solution here.

In particular because coordinating client->slave->kded->KDirNotify has context problems anyway. We'd only want to monitor directories that the client currently displays, but the slave doesn't know what they are. So we could only guess and possibly monitor too many (bad for performance on both client and server) or too few (giving unreliable updates). The only way I see that fixed is through new KIO slave API where clients (e.g. dolphin) explicitly subscribe and unsubscribe to/from dir notifications (similar to the free space info, except without polling).

Other than auto refresh, the only other option right now would be to manage multiple notification subscribers in one process each to bypass the threading problem. Which adds the complexity of managing the life time of multiple processes, the overhead of having multiple processes, each with their own smb context (which by the way also runs risk of exhausting the connection cap from what I understand; i.e. servers have a cap of how many concurrent connections they will allow a client to make) and greater risk that KIOd may forget cached credentials so we'd trigger contextless auth dialogs. Never the less this would give the much better experience. The question is whether it is worth it to us. Between the actual processes that need building from scratch and the subscription tech for notification, we are looking at a whole lot of time investment for both code and QA. The subscription problem needs solving at some point anyway though, so maybe at least that is worthwhile anyway.

@kbroulik
Copy link

The only way I see that fixed is through new KIO slave API where clients (e.g. dolphin) explicitly subscribe and unsubscribe to/from dir notifications (similar to the free space info, except without polling).

KCoreDirLister governs the watches through KDirWatch which knows what is being displayed.
But we also have this refresh issue in pseudio directories like desktop:/, trash:/, filenamesearch:/, etc which all use some kded module so the view, which only knows rewritten URLs, gets told when any of the source files change. So overall having a proper™ infrastructure for notifying KIO slave changes in general would be very useful, not just for SMB.

@hsitter
Copy link
Member

hsitter commented Feb 26, 2020

True. The problem is exacerbated for remote slaves though, what with blanket watching being substantially more expensive.

@hsitter
Copy link
Member

hsitter commented Jul 29, 2020

I was actually being ignorant. The way KDirNotify works is that KIO clients (e.g. dolphin) broadcast which dirs they are currently interested in and then any number of clients subscribe to that broadcast to implement the watching. When changes appear they in turn then again broadcast the changes and all KIO clients that are currently interested in the dir will update their cache accordingly.

With that I've actually managed to build a not too complicated prototype where there's a single smbwatcher process listening for the KIO boardcasts and then forking/killing smbnotifier sub-processes to implement the actually watching. This bypasses the threading problem at the cost of greater resource consumption but we can't do much about that - ~3-4MiB RAM per directory. It needs some major refactoring to the smb code though to isolate non-interactive authentication handling out of the slave.

@hsitter
Copy link
Member

hsitter commented Aug 3, 2020

@hsitter
Copy link
Member

hsitter commented Aug 21, 2020

This landed in kio-extras master for the 20.12 release. Notable caveat is that latest 4 directories (~dolphin tabs) will get monitored since we need to fork helper processes to bypass the threading problem with libsmbclient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants