codegooglecom / libproxy

Automatically exported from code.google.com/p/libproxy
GNU Lesser General Public License v2.1
0 stars 0 forks source link

Add note about using libproxy together with Qt or KDE #96

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Add note about using libproxy together with Qt or KDE.

The classes KConfig and KConfigGroup (following the documentation) may not
even be instanciated nor used in a thread that isn't the main thread. (I
asume they use static elements that aren't protected by a mutex.)

In consequence: The documentation note should contain something like:

"In Qt or KDE based applications, you should use libproxy only from the
main thread."

Original issue reported on code.google.com by sommer...@gmail.com on 14 Mar 2010 at 1:13

GoogleCodeExporter commented 9 years ago
I assume this will be close to impossible. Just look at the example VLC, which 
has
it's GUI based around Qt but anything stream related is in an extra thread. And 
the
http thread is the one communicating with libproxy. This would mean that we 
have a
horrible regression as a cost for the advantage of less implementation on our 
side
for using KConfig.

Original comment by dominiqu...@gmail.com on 14 Mar 2010 at 1:36

GoogleCodeExporter commented 9 years ago
Yes, that's right, the regression is quite big. See my last comments on issue 
70. For
this reason, later I was against KConfig support (counting that better KConfig
support will be relevant only for about, let's say, 1% of the users).

And the documentation of KConfig says that you may not even instanciate 
_different_
objects of this class at the same time in different threads.

Original comment by sommer...@gmail.com on 14 Mar 2010 at 4:17

GoogleCodeExporter commented 9 years ago
I'm unable to find that documentation, can you provide a link?

Original comment by npmccallum@gmail.com on 14 Mar 2010 at 7:51

GoogleCodeExporter commented 9 years ago
The documentation of KConfig at
http://api.kde.org/4.x-api/kdelibs-apidocs/kdecore/html/classKConfig.html does 
not
say nothing about that this class is thread-safe or reentrant. (It it would be
thread-safe or reentrant, than this would also be mentioned.)

I had found this snippets from the kde-devel mailing list:

http://lists.zerezo.com/kde-devel/msg12619.html
http://lists.kde.org/?l=kde-devel&m=118037451920733&w=2

They say that KConfig is neither thread-safe nor reentrant.

Qt (and so also KDE) defines the terms thread-safe and reentrant in its 
documentation
(http://doc.trolltech.com/4.6/threads-reentrancy.html) this way:

--- starting quote ---

A thread-safe function can be called simultaneously from multiple threads, even 
when
the invocations use shared data, because all references to the shared data are
serialized.

A reentrant function can also be called simultaneously from multiple threads, 
but
only if each invocation uses its own data.

Hence, a thread-safe function is always reentrant, but a reentrant function is 
not
always thread-safe.

--- end of quote ---

So, that KConfig is not even reentrant is really bad!

Furthermore, KConfig makes use of the applicationName property of 
QCoreApplication.
It produces the error message - that we supress with qInstallMsgHandler - if 
this
applicationName is not available because there is no QCoreApplication object. 
This
property is neither thread-safe nor reentrant. (The documentation at
http://qt.nokia.com/doc/4.6/qcoreapplication.html#applicationName-prop doesn't
mention thread-safety or reentrant).

Furthermore, the call of qInstallMsgHandler is not thread-safe/reentrant. (The
documentation at http://doc.trolltech.com/4.6/qtglobal.html#qInstallMsgHandler
doesn't mark it as thread-safe/reentrant, so it will not be..)

Original comment by sommer...@gmail.com on 16 Mar 2010 at 10:00

GoogleCodeExporter commented 9 years ago
Again, I would say that KConfig support is nice to have, but for about maybe 
98% of
the users, it will make no difference. So dropping KConfig support and go back 
to the
old solution would not be such a big regression.

Original comment by sommer...@gmail.com on 16 Mar 2010 at 10:03

GoogleCodeExporter commented 9 years ago
If you want KConfig support in each case, it would be an option to build a
minimalistic stand-alone helper application that just reads KDE's proxy settings
(using KConfig - as a stand-alone application this would be no problem) and 
dumps the
result. The KDE module of libproxy could simply invoke this application and 
read its
output. This way, the KDE module itself doesn't even need to link against KDE
anymore. If you want, I could write such a helper application...

Original comment by sommer...@gmail.com on 16 Mar 2010 at 10:08

GoogleCodeExporter commented 9 years ago
So I think there is some confusion here.  libproxy locks each call to 
get_proxies()
per proxy factory object.

Any code that is therefore not globally thread-safe should be called in the
constructor of the config extension.

The question I have then is this: is the kconfig object itself internally 
thread-safe
if called in a locked thread, or is it always thread-unsafe.  If it is always
thread-unsafe, we can put it in a process helper like gnome (see pxgconf).

Original comment by npmccallum@gmail.com on 16 Mar 2010 at 7:27

GoogleCodeExporter commented 9 years ago
It is always internally thread-unsafe (even the constructor)!

A process helper like gnome would be IMHO the best way (something like this I 
tried
to propose in Comment 6).

Original comment by sommer...@gmail.com on 17 Mar 2010 at 12:22

GoogleCodeExporter commented 9 years ago
Are there any news on this issue?

Original comment by sommer...@gmail.com on 8 Jun 2010 at 7:53