[prev in list] [next in list] [prev in thread] [next in thread]
List: freedesktop-dbus
Subject: [PATCHES] D-Bus C++ binding fixes
From: "Olivier Hochreutiner" <olivier.hochreutiner () gmail ! com>
Date: 2007-12-28 16:07:30
Message-ID: f967eaa30712280807u4390b243r94ce64c0f2867766 () mail ! gmail ! com
[Download RAW message or body]
Hi there,
After several tests on D-Bus C++ binding I found several bugs, and I
made fixes for some of them.
So here is what I found:
1. When a proxy is instantiated several times for a given connection,
only one instance receive signals. Patch:
fix-signals-multiple-proxies.patch
***
2. When two processes (both connected to the same bus) have an adaptor
for the same object path and interface, when a method call is done on
one of them, it is also done on the other one. For instance if you
have two 'echo-server' running (supposing they have different bus
names) and you call the method Hello of
org.freedesktop.DBus.Example.Echo on one of them (with e.g.
dbus-send), the method will also be called on the other one. Patch:
fix-method-call.patch
***
3. Threading in libdbus-c++ is never compiled due to incorrect #ifdefs
and an invalid type. Patch: pthread.patch
***
4. Even after applying patch (3), having multiple threads is not
working correctly. The problem can be easily reproduced with the
'echo-client-mt' example of D-Bus C++ binding. For recall this program
creates 16 threads. Each thread does the same method call 100 times in
a loop (method calls are done with
dbus_connection_send_with_reply_and_block()), and the main thread is
the dispatcher thread. When echo-client-mt is run, two things can
occur:
(1) It crashes with an out-of-memory error (that is, an exception is
raised because dbus_connection_send_with_reply_and_block() returns an
OOM error). I will explain this issue in another post because I
believe the problem is in libdbus-1, not libdbus-c++.
or
(2) One of the threads hangs in poll() waiting for the method reply
(see bt-thread-poll.txt), while the other threads are waiting to
acquire the io path mutex (see bt-wait-io.txt). The dispatcher thread
is usually also waiting in poll() (see bt-dispatcher-poll.txt), but it
sometimes happens that it is waiting for the io path mutex.
In this situation when the poll timeout is reached the program
continues normally until it hangs again, or crashes as in situation
(1).
I believe this problem is caused by the fact that two threads (namely
the dispatcher thread and one of the method-calling threads) poll the
same fd at the same time. The method-calling thread owns the io path
mutex while poll()'ing, but the dispatcher thread doesn't (it calls
poll() on the fd without locking anything). Sometimes the
method-calling thread returns from poll() before the dispatcher
thread, and it goes as expected, but sometimes the dispatcher thread
is notified first, resulting in the method-calling thread having to
wait until poll times out, and only then it returns from
dbus_connection_send_with_reply_and_block(). Of course all the other
threads have to wait for the io path mutex.
A workaround for this issue could be to make the dispatcher thread
leave poll() before calling
dbus_connection_send_with_reply_and_block() or
dbus_bus_{add,remove}_match(). I guess this can be done by creating a
pipe, adding its read fd to the list of fds the dispatcher polls, and
writing something to the pipe's write fd before calling the
above-mentionned functions. Probably a mutex or a condition variable
would be needed too, to tell the dispatcher when the method call is
finished.
Is this solution acceptable ? Should the issue rather be fixed somehow
in libdbus-1 ? What do you think ?
***
Beside these issues there are two things that I would like to ask:
- I think D-Bus C++ sources should be moved out of Wengo's SVN. I
believe it was already discussed on this list that it can be moved to
git.freedesktop.org, using
http://www.kernel.org/pub/software/scm/git/docs/git-svnimport.html. Is
it still planned ?
- What about relicensing D-Bus C++ with the same license as dbus-1
(i.e. MIT/X11) ?
Well I think that's all. Merry Christmas and Happy New Year to everybody.
Olivier Hochreutiner
["fix-method-call.patch" (text/x-patch)]
Index: dbus/src/object.cpp
===================================================================
--- dbus/src/object.cpp (revision 13253)
+++ dbus/src/object.cpp (working copy)
@@ -143,7 +143,7 @@
InterfaceAdaptorTable::const_iterator ii = _interfaces.begin();
while( ii != _interfaces.end() )
{
- std::string im = \
"type='method_call',interface='"+ii->first+"',path='"+path()+"'"; + std::string im \
= "type='method_call',interface='"+ii->first+"',path='"+path()+"',destination='"+(conn().names())[0]+"'";
conn().add_match(im.c_str());
++ii;
}
@@ -163,7 +163,7 @@
InterfaceAdaptorTable::const_iterator ii = _interfaces.begin();
while( ii != _interfaces.end() )
{
- std::string im = "type='method_call',interface='"+ii->first+"',path='"+path()+"'";
+ std::string im = "type='method_call',interface='"+ii->first+"',path='"+path()+"',destination='"+(conn().names())[0]+"'";
conn().remove_match(im.c_str());
++ii;
}
["pthread.patch" (text/x-patch)]
Index: dbus/include/dbus-c++/eventloop.h
===================================================================
--- dbus/include/dbus-c++/eventloop.h (revision 13253)
+++ dbus/include/dbus-c++/eventloop.h (working copy)
@@ -29,7 +29,7 @@
#include "config.h"
#endif
-#ifdef HAVE_PTHREAD
+#ifdef HAVE_PTHREAD_H
#include <pthread.h>
#endif
@@ -143,9 +143,9 @@
private:
-#if defined HAVE_PTHREAD
+#if defined HAVE_PTHREAD_H
- pthread_mutex _mutex;
+ pthread_mutex_t _mutex;
#elif defined HAVE_WIN32
Index: dbus/src/eventloop.cpp
===================================================================
--- dbus/src/eventloop.cpp (revision 13253)
+++ dbus/src/eventloop.cpp (working copy)
@@ -74,7 +74,7 @@
DefaultMutex::DefaultMutex()
{
-#if defined HAVE_PTHREAD
+#if defined HAVE_PTHREAD_H
pthread_mutex_init(&_mutex, NULL);
@@ -84,7 +84,7 @@
DefaultMutex::~DefaultMutex()
{
-#if defined HAVE_PTHREAD
+#if defined HAVE_PTHREAD_H
pthread_mutex_destroy(&_mutex);
@@ -94,7 +94,7 @@
void DefaultMutex::lock()
{
-#if defined HAVE_PTHREAD
+#if defined HAVE_PTHREAD_H
pthread_mutex_lock(&_mutex);
@@ -104,7 +104,7 @@
void DefaultMutex::unlock()
{
-#if defined HAVE_PTHREAD
+#if defined HAVE_PTHREAD_H
pthread_mutex_unlock(&_mutex);
["fix-signals-multiple-proxies.patch" (text/x-patch)]
Index: dbus/src/interface.cpp
===================================================================
--- dbus/src/interface.cpp (revision 13253)
+++ dbus/src/interface.cpp (working copy)
@@ -131,7 +131,11 @@
if( si != _signals.end() )
{
si->second.call( msg );
- return true;
+ // Here we always return false because there might be
+ // another InterfaceProxy listening for the same signal.
+ // This way we instruct libdbus-1 to go on dispatching
+ // the signal.
+ return false;
}
else
{
["bt-thread-poll.txt" (text/plain)]
#0 0xffffe410 in __kernel_vsyscall ()
#1 0xb7ce25e7 in poll () from /lib/tls/i686/cmov/libc.so.6
#2 0xb7bfdcd1 in _dbus_poll (fds=0xb7b58020, n_fds=1, timeout_milliseconds=2000) at \
dbus-sysdeps-unix.c:1850 #3 0xb7bdb3db in socket_do_iteration (transport=0x8051df8, \
flags=6, timeout_milliseconds=2000) at dbus-transport-socket.c:1042 #4 0xb7bd9247 in \
_dbus_transport_do_iteration (transport=0x8051df8, flags=6, \
timeout_milliseconds=2000) at dbus-transport.c:941 #5 0xb7ba1f2f in \
_dbus_connection_do_iteration_unlocked (connection=0x80521a0, flags=6, \
timeout_milliseconds=2000) at dbus-connection.c:1143 #6 0xb7ba45f5 in \
_dbus_connection_block_pending_call (pending=0x8055218) at dbus-connection.c:2265 #7 \
0xb7bcc669 in dbus_pending_call_block (pending=0x8055218) at dbus-pending-call.c:707 \
#8 0xb7ba70e6 in dbus_connection_send_with_reply_and_block (connection=0x80521a0, \
message=0x8052450, timeout_milliseconds=2000, error=0xb7b58198) at \
dbus-connection.c:3267 #9 0xb7ed1d82 in DBus::Connection::send_blocking \
(this=0xb7b5835c, msg=@0xb7b58258, timeout=2000) at connection.cpp:349 #10 0xb7ec85fb \
in DBus::ObjectProxy::_invoke_method (this=0xb7b58358, call=@0xb7b58258) at \
object.cpp:318 #11 0xb7ec2897 in DBus::InterfaceProxy::invoke_method \
(this=0xb7b58318, call=@0xb7b58258) at interface.cpp:147 #12 0x0804cf64 in \
org::freedesktop::DBus::EchoDemo::Hello (this=0xb7b58318, name=@0xb7b583a0) at \
echo-client-glue.h:50 #13 0x0804adfb in greeter_thread (arg=0xbf9a9f48) at \
echo-client.cpp:45 #14 0xb7e8c46b in start_thread () from \
/lib/tls/i686/cmov/libpthread.so.0 #15 0xb7cec6de in clone () from \
/lib/tls/i686/cmov/libc.so.6
["bt-wait-io-path.txt" (text/plain)]
#0 0xffffe410 in __kernel_vsyscall ()
#1 0xb7e90676 in pthread_cond_wait@@GLIBC_2.3.2 () from \
/lib/tls/i686/cmov/libpthread.so.0 #2 0xb7bfa6a5 in _dbus_pthread_condvar_wait \
(cond=0x80522a8, mutex=0x8052258) at dbus-sysdeps-pthread.c:231 #3 0xb7bd6a70 in \
_dbus_condvar_wait (cond=0x80522a8, mutex=0x8052258) at dbus-threads.c:254 #4 \
0xb7ba1bad in _dbus_connection_acquire_io_path (connection=0x80521a0, \
timeout_milliseconds=-1) at dbus-connection.c:1044 #5 0xb7ba1ecc in \
_dbus_connection_do_iteration_unlocked (connection=0x80521a0, flags=7, \
timeout_milliseconds=-1) at dbus-connection.c:1138 #6 0xb7ba723d in \
_dbus_connection_flush_unlocked (connection=0x80521a0) at dbus-connection.c:3311 #7 \
0xb7ba44a9 in _dbus_connection_block_pending_call (pending=0x8055648) at \
dbus-connection.c:2236 #8 0xb7bcc669 in dbus_pending_call_block (pending=0x8055648) \
at dbus-pending-call.c:707 #9 0xb7ba70e6 in \
dbus_connection_send_with_reply_and_block (connection=0x80521a0, message=0x8056628, \
timeout_milliseconds=2000, error=0xb7357198) at dbus-connection.c:3267 #10 0xb7ed1d82 \
in DBus::Connection::send_blocking (this=0xb735735c, msg=@0xb7357258, timeout=2000) \
at connection.cpp:349 #11 0xb7ec85fb in DBus::ObjectProxy::_invoke_method \
(this=0xb7357358, call=@0xb7357258) at object.cpp:318 #12 0xb7ec2897 in \
DBus::InterfaceProxy::invoke_method (this=0xb7357318, call=@0xb7357258) at \
interface.cpp:147 #13 0x0804cf64 in org::freedesktop::DBus::EchoDemo::Hello \
(this=0xb7357318, name=@0xb73573a0) at echo-client-glue.h:50 #14 0x0804adfb in \
greeter_thread (arg=0xbf9a9f48) at echo-client.cpp:45 #15 0xb7e8c46b in start_thread \
() from /lib/tls/i686/cmov/libpthread.so.0 #16 0xb7cec6de in clone () from \
/lib/tls/i686/cmov/libc.so.6
["bt-dispatcher-poll.txt" (text/plain)]
#0 0xffffe410 in __kernel_vsyscall ()
#1 0xb7ce25e7 in poll () from /lib/tls/i686/cmov/libc.so.6
#2 0xb7edbd2e in DBus::DefaultMainLoop::dispatch (this=0x8050cc4) at eventloop.cpp:183
#3 0xb7ede81d in DBus::BusDispatcher::do_iteration (this=0x8050ca0) at eventloop-integration.cpp:90
#4 0xb7ede4df in DBus::BusDispatcher::enter (this=0x8050ca0) at eventloop-integration.cpp:76
#5 0x0804ab3e in main () at echo-client.cpp:84
_______________________________________________
dbus mailing list
dbus@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dbus
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic