> On Sept. 11, 2015, 12:56 p.m., Albert Astals Cid wrote:
> > src/kbuildsycoca/kbuildsycoca.cpp, line 370
> > <https://git.reviewboard.kde.org/r/125152/diff/1/?file=402639#file402639line370>
> >
> >     .lock() calls scare me, this will block forever if say kbuildsycoca 
> > crashed, right?
> >     
> >     How do we recover from that? Will this also lock the UI when we move 
> > the sycoca creation to apps?
> 
> David Edmundson wrote:
>     QLockFile writes a timestamp and PID  into the lockfile. This is checked 
> when the next process checks the lock.
> 
> Albert Astals Cid wrote:
>     Ah nice, then just have the question if "Will this also lock the UI when 
> we move the sycoca creation to apps?"

Let me answer all these questions in order :-)

1) .lock() will not block *forever* if kbuildsycoca crashed, it will wait at 
most 30 seconds, in the worst case where the PID got reused meanwhile. The more 
likely case is that the PID is no longer in use, in which case QLockFile will 
detect immediately that the lock file is stale, and delete it.

2) Yes this code will block the GUI if called from a GUI application. But note 
that we already do exactly that: ksycoca currently calls (in 
KSycocaPrivate::buildSycoca) QProcess::waitForFinished in order to wait for 
kbuildsycoca to be done before reopening the DB. We've had that forever. The 
apps do need the stuff, in synchronous calls (e.g. KMimeTypeTrader) so they 
have to wait for the sycoca db to exist. OK, with one difference though, apps 
only used to do this when ksycoca didn't exist at all or its version number was 
too low (i.e. after upgrading the lib). With my changes for the last month, 
this would also happen when new desktop files have been installed. That is the 
bugfix for the 15 years old issues with sycoca missing some stuff...
I think the app blocking (for a tiny bit of time, this is an incremental 
rebuild) in order to see the new desktop files is actually an improvement, and 
there's not really another choice anyway. And merging kbuildsycoca into the lib 
will make the rebuild faster, compared to QProcess + relocations + 
initializations + loading the existing ksycoca in the kbuildsycoca process... 
all of that will go away.

Summary: we'll still block, more often (mostly for developers), but for a 
smaller amount of time.

d_ed: the lock file contains PID, appname, hostname. The timestamp used for the 
30s check is the file's modification time, simply.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125152/#review85178
-----------------------------------------------------------


On Sept. 11, 2015, 7:23 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125152/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2015, 7:23 a.m.)
> 
> 
> Review request for KDE Frameworks and Albert Astals Cid.
> 
> 
> Repository: kservice
> 
> 
> Description
> -------
> 
> This is a more traditional way to protect concurrent access to a file,
> and it will (more easily) allow to have one lock file per sycoca file
> when we have LANG and DIRS in the filename.
> 
> It doesn't exactly remove the dependency on DBus though, since we still
> use it for the notification after the rebuild.
> 
> As an additional improvement, if a process has to wait for another
> to rebuild sycoca, when it can finally get the lock file, it does a quick
> mtime check and if there were no additional changes compared to the
> time of the last check (as stored in ksycoca's global header) then there
> is nothing to do, we can do an early return.
> 
> 
> Diffs
> -----
> 
>   src/kbuildsycoca/kbuildsycoca.cpp 2a1c6b3e2ef2add82f710e1a18cf77028f437407 
>   src/kbuildsycoca/kbuildsycoca_main.cpp 
> 5a0c0e8b47996750a1bf25139e1c21eb0392044b 
>   src/kbuildsycoca/kbuildsycoca_p.h 1be91bf21bb6908a63ffe6c156830838ed1ff429 
>   src/sycoca/ksycoca.h 9d8b21e3c0f08375bece923c4029b54617f04b7f 
>   src/sycoca/ksycoca.cpp 36718e3ee951df19494031f17dec29d1f4dd39c5 
>   src/sycoca/ksycoca_p.h 1a377e3586330a22cbcab507ea7d7f16d1563f13 
> 
> Diff: https://git.reviewboard.kde.org/r/125152/diff/
> 
> 
> Testing
> -------
> 
> Running "kbuildsycoca --noincremental &" 5 times in a row quickly, and 
> watching the debug output, shows that only the first one rebuilds, all others 
> wait (approx 1.5s) and then realize there is nothing more to do, and exit.
> 
> 
> Thanks,
> 
> David Faure
> 
>

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to