On Di, 2011-11-29 at 13:20 +0100, Chris Kühl wrote:
> On Tue, Nov 29, 2011 at 12:12 PM, Patrick Ohly <[email protected]> wrote:
> > On Mo, 2011-11-28 at 12:24 +0100, Chris Kühl wrote:
> >> On Wed, Nov 16, 2011 at 2:52 PM, Patrick Ohly <[email protected]>
> >> wrote:
> >> > On Mi, 2011-11-16 at 11:58 +0100, Patrick Ohly wrote:
> >> >> On Mi, 2011-11-16 at 11:10 +0100, Chris Kühl wrote:
> >> >> > On Mon, Nov 14, 2011 at 4:49 PM, Chris Kühl <[email protected]> wrote:
> >> >> > > On Mon, Nov 14, 2011 at 2:31 PM, Patrick Ohly
> >> >> > > <[email protected]> wrote:
> >> >> > > Ok. We should be close to requesting a merge. Only 2 failures to fix
> >> >> > > before we've reached parity on our machines.
> >> >> > >
> >> >> >
> >> >> > After fixing those remaining issues, we ran the server under valgrind
> >> >> > and fixed up a few memory leaks we found. I've now created a
> >> >> > for-master/gio-gdbus branch, squashed the commits and pushed.
> >> >>
> >> >> I've started a test run.
> >> >
> >> > There seems to be a regression in the D-Bus testing.
> >> >
> >> > On Ubuntu Lucid, startup of the syncevo-dbus-server without GIO GDBus
> >> > fails:
> >> > http://syncev.meego.com/2011-11-16-10-54_dist/lucid-amd64/6-dbus/output.txt
> >> >
> >>
> >> So, I finally got the issue in Debian Stable and Ubuntu LTS fixed.
> >> Turned out to be that I'd inadvertently set the server's main
> >> connection to shared instead of private. I'm not sure why this was
> >> only causing issues on these distros. That initially sent me searching
> >> for the issue in all the wrong places.
> >
> > lucid-amd64 without GIO GDBus passed the tests now. I had to take the
> > for-master/gio-gdbus branch temporarily out of testing because the
> > autoconf changes conflicted with another branch, but now it is back and
> > was tested.
> >
> > However, it segfaults during startup on Debian Testing with GIO GDBus
> > enabled:
> > http://syncev.meego.com/latest/testing-amd64-gio/6-dbus/output.txt
> >
>
> Link is broken.
Ah, the "latest" symlink. I need to remember not to use that in email...
> But I've not had that issue with Debian Testing. Hmm?
> I've got a clean install with only the necessary packages added for
> building and running syncevolution.
It happens when Bluez is not running.
Program received signal SIGSEGV, Segmentation fault.
0x0000000000597e36 in set (error=0xb7e4c0, this=0x0) at
/data/runtests/work/sources/syncevolution/src/gdbusxx/gdbus-cxx-bridge.h:164
warning: Source file is more recent than executable.
164 if (m_error) {
(gdb) where
#0 0x0000000000597e36 in set (error=0xb7e4c0, this=0x0) at
/data/runtests/work/sources/syncevolution/src/gdbusxx/gdbus-cxx-bridge.h:164
#1 GDBusCXX::dbus_get_bus_connection (busType=<optimized out>, name=0x0,
unshared=<optimized out>, err=0x0) at
/data/runtests/work/sources/syncevolution/src/gdbusxx/gdbus-cxx-bridge.cpp:68
#2 0x0000000000574450 in SyncEvo::BluezManager::BluezManager (this=0xb85740,
server=<optimized out>) at
/data/runtests/work/sources/syncevolution/src/dbus/server/bluez-manager.cpp:44
#3 0x0000000000541250 in SyncEvo::Server::Server (this=0x7fffffffd9a0,
loop=0xb6eab0, shutdownRequested=@0xb46af8, restart=..., conn=<optimized out>,
duration=600)
at /data/runtests/work/sources/syncevolution/src/dbus/server/server.cpp:227
#4 0x000000000052121c in main (argc=1, argv=0x7fffffffdfa8, envp=<optimized
out>) at /data/runtests/work/sources/syncevolution/src/dbus/server/main.cpp:113
GDBusCXX::dbus_get_bus_connection() takes a GDBusCXX::DBusErrorCXX *err
parameter and doesn't check it for NULL:
63 } else {
64 // This returns a singleton, shared connection object.
65 conn = g_bus_get_sync(boost::iequals(busType, "SESSION") ?
G_BUS_TYPE_SESSION : G_BUS_TYPE_SYSTEM,
66 NULL, &error);
67 if(error != NULL) {
68 err->set(error); <======
69 return NULL;
70 }
71 }
Here's a patch which fixes the issue:
Modified src/gdbusxx/gdbus-cxx-bridge.cpp
diff --git a/src/gdbusxx/gdbus-cxx-bridge.cpp b/src/gdbusxx/gdbus-cxx-bridge.cpp
index f5194e7..b103509 100644
--- a/src/gdbusxx/gdbus-cxx-bridge.cpp
+++ b/src/gdbusxx/gdbus-cxx-bridge.cpp
@@ -35,7 +35,7 @@ boost::function<void (void)> MethodHandler::m_callback;
GDBusConnection *dbus_get_bus_connection(const char *busType,
const char *name,
bool unshared,
- DBusErrorCXX *err /* Ignored */)
+ DBusErrorCXX *err)
{
GDBusConnection *conn;
GError* error = NULL;
@@ -45,7 +45,9 @@ GDBusConnection *dbus_get_bus_connection(const char *busType,
G_BUS_TYPE_SESSION :
G_BUS_TYPE_SYSTEM,
NULL, &error);
if(address == NULL) {
- err->set(error);
+ if (err) {
+ err->set(error);
+ }
return NULL;
}
// Here we set up a private client connection using the chosen bus'
address.
@@ -56,24 +58,24 @@ GDBusConnection *dbus_get_bus_connection(const char
*busType,
NULL, NULL, &error);
g_free(address);
- if(error != NULL) {
- err->set(error);
+ if(conn == NULL) {
+ if (err) {
+ err->set(error);
+ }
return NULL;
}
} else {
// This returns a singleton, shared connection object.
conn = g_bus_get_sync(boost::iequals(busType, "SESSION") ?
G_BUS_TYPE_SESSION : G_BUS_TYPE_SYSTEM,
NULL, &error);
- if(error != NULL) {
- err->set(error);
+ if(conn == NULL) {
+ if (err) {
+ err->set(error);
+ }
return NULL;
}
}
- if(!conn) {
- return NULL;
- }
-
if(name) {
g_bus_own_name_on_connection(conn, name, G_BUS_NAME_OWNER_FLAGS_NONE,
NULL, NULL, NULL, NULL);
Does that look right?
Speaking of DBusErrorCXX, why does it have a separate "message" member?
/**
* wrapper around GError which initializes
* the struct automatically, then can be used to
* throw an exception
*/
class DBusErrorCXX
{
GError *m_error;
public:
char *message;
DBusErrorCXX(GError *error = NULL)
: m_error(error)
{
if (m_error) {
message = m_error->message;
}
}
That "message" pointer is not getting reset when set(NULL) is called,
leading to a dangling pointer:
void set(GError *error) {
if (m_error) {
g_error_free (m_error);
}
m_error = error;
if (m_error) {
message = m_error->message;
}
}
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
_______________________________________________
SyncEvolution mailing list
[email protected]
http://lists.syncevolution.org/listinfo/syncevolution
