rjvbb abandoned this revision.
REPOSITORY
R244 KCoreAddons
REVISION DETAIL
https://phabricator.kde.org/D8043
To: rjvbb, #frameworks, mwolff, dfaure
Cc: dhaumann, cfeck, dfaure, mwolff, kde-frameworks-devel
rjvbb added a comment.
OK, my bad, `fsWatcher` is a `KDirWatchPrivate` member, not a global and not
a static member either. Apologies for that - but it doesn't change the symptoms
I'm seeing.
REPOSITORY
R244 KCoreAddons
REVISION DETAIL
https://phabricator.kde.org/D8043
To: rjvbb,
rjvbb added a comment.
Hmmm, I'll double-check. I didn't really go beyond the thought that it seemed
to make sense to use a single watcher instance if most of the time you'll be
watching items that live on a single disk.
> Could it be that QFSW itself uses a single static object
rjvbb added a comment.
I did see some symptoms that could only be explained by a memory layout
change (and apparently caused by this patch). Shame I didn't notice that before
posting (I only tested with recompiled code).
Would it be possible to change the name of this virtualised class
dfaure added a comment.
> Using a KDirWatch per thread would still mean that they can be accessing
the shared QFSW instance concurrently.
Are you sure it's shared? d->fsWatcher isn't static.
Could it be that QFSW itself uses a single static object internally? In that
case it's QSFW
dhaumann added a comment.
@rjvbb you are changing the memory layout of instances of this class. All
applications that use this class / library will break without recompiling.
The KDE Frameworks guarantee binary compatibility backwards. See:
cfeck added a comment.
ABI is broken for any derived class, because the added virtual increases the
size of the object. The changed offsets would require all code that uses the
derived class to be recompiled.
REPOSITORY
R244 KCoreAddons
REVISION DETAIL
https://phabricator.kde.org/D8043
rjvbb added a comment.
In a (big) nutshell: https://phabricator.kde.org/D7995.
KDevelop currently uses a single KDirWatch per open project that is fed with
a single `addDir` call which adds all folders under the project directory (and
all files, which it shouldn't).
This is fine on
rjvbb added a comment.
TBH I was indeed expecting all kinds of hell raining down when I rebuilt the
framework with this change, in the form of mysterious linker errors about
virtual thunks and what have you.
Not so, no symbols are missing from libKF5CoreAddons because of this patch,
so
dfaure requested changes to this revision.
dfaure added a comment.
In addition to being BIC, this is not the way to add thread safety. Either
the class should be threadsafe (and then it should use mutexes internally) or
it doesn't need to be threadsafe (and that can just be documented).
mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.
this is not abi compatible, cannot be done -2
REPOSITORY
R244 KCoreAddons
REVISION DETAIL
https://phabricator.kde.org/D8043
To: rjvbb, #frameworks, mwolff
Cc: mwolff,
rjvbb set the repository for this revision to R244 KCoreAddons.
rjvbb added a subscriber: kde-frameworks-devel.
REPOSITORY
R244 KCoreAddons
REVISION DETAIL
https://phabricator.kde.org/D8043
To: rjvbb, #frameworks
Cc: kde-frameworks-devel
rjvbb updated this revision to Diff 20056.
rjvbb edited the test plan for this revision.
rjvbb added a comment.
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8043?vs=20046=20056
REVISION DETAIL
https://phabricator.kde.org/D8043
AFFECTED FILES
src/lib/io/kdirwatch.h
To:
rjvbb created this revision.
rjvbb added a reviewer: Frameworks.
rjvbb added a project: Frameworks.
REVISION SUMMARY
KDirWatch is not current thread-safe which can cause problems when feeding or
deleting directories in a multi-threaded approach. In addition, the class uses
a single
14 matches
Mail list logo
