kioworkers would become libraries, and dynamically loaded into a thread of the calling application instead of a new process. (solving the android /whatever issue).
a local socket would still be used for client communication so hopefully not changed too much.
Probably needs some further investigation and research.
| Status | Assigned | Task | ||
|---|---|---|---|---|
| Open | None | T12089 KIO for KF6 | ||
| Open | None | T12214 Move KIOWorkers into a thread of the calling application instead of separate process |
I'd like this, would make debugging a million times easier to. Probably faster as well...
But this also means we no longer have process isolation. May be important for everything using third party libraries, e.g. the thumbnailer, and also probably metadata in general (see https://invent.kde.org/network/kio-extras/-/merge_requests/51)
I remember @dfaure explored this idea at KIO bof in akademy 2019.
We should be picky which ioslave could use a thread instead of a process, especially regarding stability concerns.
A "thread mode" where KIO would instanciate ioslave in threads akin to KDE_FORK_SLAVES could be a nice first step permetting the easy debug use case.
Another approach is to have a thread inside the process as I did for the DeleteJob https://phabricator.kde.org/D24962
Would this mean that if Dolphin crashed during file operations, the operation could no longer be completed, and leave partial files?
Yes, but the assumption is dolphin/filemanagers are relatively more stable than some io workers.
We might want to keep fine grain control over which ones are run in a thread. For instance nfs/smb file operation could be done in a process, but kio_files could be run in threads.
BTW the main use case is Android where forking processes is not allowed. We could still keep most slaves out of process on desktop platforms, for the robustness reason mentioned above.
I looked a bit more into this. One hairy issue that prevents doing clean refactorings of the Slave class, is the fact that Slave (and its base class SlaveInterface) are public API.
If pim/kdepim-runtime/resources/pop3/jobs.cpp could be ported away from KIO::Slave (e.g. by merging kio_pop3 into the pop3 resource), then we could remove the getConnectedSlave() API from KIO::Scheduler and un-export Scheduler, Slave and SlaveInterface. Then it would be much easier to have a proper design with Slave being a base class for ProcessSlave, ThreadSlave (and DataSlave, as already exists). Oh, and s/Slave/Worker/ everywhere for political correctness :)
BTW the main use case is Android where forking processes is not allowed.
Just for the record I'm not 100% sure this is correct. At some point I tried to get KIO with forked workers on Android but couldn't, but I can't rule out that this was due to issues on our side. However while researching that I found information that there appear to be plans to disallow forking, so in-process most likely is still the way to go, also for performance and complexity reasons
I made some more progress with this, adding a WorkerFactory base class and making kio_file's plugin QObject implement that, to create FileProtocol instances in-process.
Something I just realized: one thing we'll lose when using threads instead of processes is the ability to completely kill a running slave. Threads can't and mustn't be killed/terminated, so I guess the best thing we'll be able to do, when killing a job, is to disconnect from the worker object and let it finish its course. For some cases where it'll be worth it, like aborting large downloads/copies, we should of course then add an atomic bool, and check it in the main while loop. In separate-process mode, that bool will just never be set to true.
In other news, I just ran jobtest with kio_file in a thread, for the first time :) MR coming up.
There is no way around it, but that's fine. We already have notions of killed state. In ioworker, kio_file copy for instance. SlaveBase::wasKilled can become this atomic bool.
We should make sure the worker for which we enable thread processing do handle wasKilled case whenever we run a long operation where possible.
Still Job can choose to shadow somewhat the end of life of background threads.
In other news, I just ran jobtest with kio_file in a thread, for the first time :) MR coming up.
Great, Can't wait to see this. \o/
Good point about wasKilled already being checked by existing code, that's good.
https://invent.kde.org/frameworks/kio/-/merge_requests/740 is up
Would this mean that if Dolphin crashed during file operations, the operation could no longer be completed, and leave partial files?
Note that already today, if dolphin crashes in the middle of copying N items, or a directory, the operation won't complete, since the Dolphin process (in libkio) orchestrates the whole thing. Only copying one large file would finish on its own in the background.