| Summary: | dbus_shutdown() called but connections were still live | ||
|---|---|---|---|
| Product: | dbus | Reporter: | frederic heem <frederic.heem> |
| Component: | core | Assignee: | Havoc Pennington <hp> |
| Status: | RESOLVED FIXED | QA Contact: | John (J5) Palmieri <johnp> |
| Severity: | normal | ||
| Priority: | high | CC: | jos, kimmo.hamalainen, rob.taylor |
| Version: | unspecified | Keywords: | patch |
| Hardware: | Other | ||
| OS: | Linux (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
proposed patch
fixed patch |
||
|
Description
frederic heem
2006-09-21 03:30:34 UTC
Make sure you call dbus_connection_close() on your connections (not the shared ones). Make sure you call dbus_connection_unref() on the shared ones. That was already done, here is the destructor dbus_g_connection_flush(pSniffer->pConnection); dbus_g_connection_unref(pSniffer->pConnection); dbus_shutdown(); Note that dbus_g_connection_close and dbus_g_shutdown() don't exist. The simplest possible explanation is a reference count leak somewhere, can you look at the refcount in the connection in the debugger (break on dbus_shutdown) and see what it is, then track down where any existing count came from... ? Before calling dbus_g_connection_unref, refcount is 3. Setting a breakpoint on dbus_g_connection_ref shows who is the culprit, here are the 3 backtraces: First backtrace #0 dbus_connection_ref (connection=0x936dfb0) at dbus-connection.c:1743 #1 0x007fe5dc in internal_bus_get (type=DBUS_BUS_SYSTEM, error=0xbfd2403c, private=0) at dbus-bus.c:419 #2 0x00bbd539 in dbus_g_bus_get (type=DBUS_BUS_SYSTEM, error=0xbfd24098) at dbus-gmain.c:769 #3 0x0805230a in sniffer_dbus_register_service (pSniffer=0x936ac00) at dbus/wireshark-dbus-sniffer.c:352 #4 0x080530f5 in sniffer_dbus_init (pCaptureOptions=0x8058c80) at dbus/wireshark-dbus-sniffer.c:252 #5 0x080504f1 in main (argc=2, argv=0xbfd241d4) at dumpcap.c:381 Second backtrace: #0 dbus_connection_ref (connection=0x936dfb0) at dbus-connection.c:1743 #1 0x00bbc2f5 in dbus_g_connection_ref (gconnection=0x936dfb4) at dbus-glib.c:64 #2 0x0014d2d2 in boxed_proxy_collect_value (value=0x937127c, n_collect_values=1, collect_values=0xbfd23f9c, collect_flags=0) at gboxed.c:306 #3 0x00154e5d in IA__g_object_new_valist (object_type=154577104, first_property_name=0xbd2076 "name", var_args=0xbfd24044 "") at gobject.c:984 #4 0x00154fe0 in IA__g_object_new (object_type=154577104, first_property_name=0xbd2076 "name") at gobject.c:793 #5 0x00bc4308 in dbus_g_proxy_new (connection=0x936dfb4, name=0x80569a9 "org.freedesktop.DBus", path_name=0x80569be "/org/freedesktop/DBus", interface_name=0x80569a9 "org.freedesktop.DBus") at dbus-gproxy.c:1824 #6 0x08052358 in sniffer_dbus_register_service (pSniffer=0x936ac00) at dbus/wireshark-dbus-sniffer.c:367 #7 0x080530f5 in sniffer_dbus_init (pCaptureOptions=0x8058c80) at dbus/wireshark-dbus-sniffer.c:252 #8 0x080504f1 in main (argc=2, argv=0xbfd241d4) at dumpcap.c:381 Third backtrace: #0 dbus_connection_ref (connection=0x936dfb0) at dbus-connection.c:1743 #1 0x00bbc2f5 in dbus_g_connection_ref (gconnection=0x936dfb4) at dbus-glib.c:64 #2 0x0014d390 in boxed_proxy_value_copy (src_value=0x937127c, dest_value=0xbfd23dd8) at gboxed.c:273 #3 0x001723fa in IA__g_value_copy (src_value=0x937127c, dest_value=0xbfd23dd8) at gvalue.c:119 #4 0x001724ff in IA__g_value_transform (src_value=0x937127c, dest_value=0xbfd23dd8) at gvalue.c:341 #5 0x0015683d in g_object_constructor (type=154577104, n_construct_properties=4, construct_params=0x9371438) at gobject.c:677 #6 0x00bc5c4e in dbus_g_proxy_constructor (type=154577104, n_construct_properties=4, construct_properties=0x9371418) at dbus-gproxy.c:1311 #7 0x0015432a in IA__g_object_newv (object_type=154577104, n_parameters=4, parameters=0x9371230) at gobject.c:912 #8 0x00154ed9 in IA__g_object_new_valist (object_type=154577104, first_property_name=0xbd2076 "name", var_args=0xbfd24044 "") at gobject.c:996 #9 0x00154fe0 in IA__g_object_new (object_type=154577104, first_property_name=0xbd2076 "name") at gobject.c:793 #10 0x00bc4308 in dbus_g_proxy_new (connection=0x936dfb4, name=0x80569a9 "org.freedesktop.DBus", path_name=0x80569be "/org/freedesktop/DBus", interface_name=0x80569a9 "org.freedesktop.DBus") at dbus-gproxy.c:1824 #11 0x08052358 in sniffer_dbus_register_service (pSniffer=0x936ac00) at dbus/wireshark-dbus-sniffer.c:367 #12 0x080530f5 in sniffer_dbus_init (pCaptureOptions=0x8058c80) at dbus/wireshark-dbus-sniffer.c:252 #13 0x080504f1 in main (argc=2, argv=0xbfd241d4) at dumpcap.c:381 The actuall explination is we hold a ref on the shared connection. Assert should be changed to connection->refcount <= 1 or connection->refcount == 1 since it actually be an error if it is == 0 and still in the cache. Actually is is a little more complicated than that. Since we are talking about shared known connections (i.e. SESSION, SYSTEM and STARTER bus) we might have two refs, one for the shared connection cache and one for the known bus array. Not sure what the best course of action is but perhaps have the shutdown functions unref and then have a check afterwards. *** Bug 8377 has been marked as a duplicate of this bug. *** *** Bug 8367 has been marked as a duplicate of this bug. *** shutdown will need to disconnect the shared connections and drop refs to them... havoc, did one of your commits fix this? Yes, this is probably fixed now. I changed the dbus-bus.c ref to be a weak ref. Havoc says fixed. Please reopen if it is not The bug still exists in the RC2. the only real way to debug this is to write a simple compilable test case program, anyone want to do that ;-) It could even go in the test suite, it would be useful to just run a little app that opens a bus connection, calls some methods and sends some signals, then dbus_shutdown(), just to verify that the shutdown works and nothing crashes. It is actually written but we clean up the connections ourselves with test_connection_shutdown so we never hit the case. Taking test_connection_shutdown out of test/test-service.c should give us the testcase we need. Actually I can't reproduce this with RC2 (in fact I can ref a connection numerous time and it won't complain when dbus_shutdown is called). Please file a test case with expected and actuall results placed in the comments. The problem seems to be in glib binding. dbus_g_proxy_new is calling 2 times dbus_g_connection_ref Here is the 2 backtrace when setting a breakpoint in dbus_g_connection_ref: bt 1: #0 dbus_g_connection_ref (gconnection=0x97d5bf4) at dbus-glib.c:66 #1 0x0014d2d2 in boxed_proxy_collect_value (value=0x97d7dac, n_collect_values=1, collect_values=0xbf9638cc, collect_flags=0) at gboxed.c:306 #2 0x00154e5d in IA__g_object_new_valist (object_type=159204112, first_property_name=0x5ae10e "name", var_args=0xbf963974 "") at gobject.c:984 #3 0x00154fe0 in IA__g_object_new (object_type=159204112, first_property_name=0x5ae10e "name") at gobject.c:793 #4 0x005a0358 in dbus_g_proxy_new (connection=0x97d5bf4, name=0x649f3f "org.freedesktop.DBus", path_name=0x649f54 "/org/freedesktop/DBus", interface_name=0x649f3f "org.freedesktop.DBus") at dbus-gproxy.c:1824 #5 0x00648247 in sniffer_dbus_register_service (pSniffer=0x97d42c8) at /home/heefre/software/dbus/wireshark/dbus/wireshark-dbus-sniffer.c:373 #6 0x00647ded in sniffer_dbus_init (pCaptureOptions=0x80553e0) at /home/heefre/software/dbus/wireshark/dbus/wireshark-dbus-sniffer.c:255 #7 0x0804fe29 in main (argc=2, argv=0xbf963b14) at /home/heefre/software/dbus/wireshark/dumpcap.c:381 bt2: #0 dbus_g_connection_ref (gconnection=0x97d5bf4) at dbus-glib.c:66 #1 0x0014d390 in boxed_proxy_value_copy (src_value=0x97d7dac, dest_value=0xbf963708) at gboxed.c:273 #2 0x001723fa in IA__g_value_copy (src_value=0x97d7dac, dest_value=0xbf963708) at gvalue.c:119 #3 0x001724ff in IA__g_value_transform (src_value=0x97d7dac, dest_value=0xbf963708) at gvalue.c:341 #4 0x0015683d in g_object_constructor (type=159204112, n_construct_properties=4, construct_params=0x97d5f80) at gobject.c:677 #5 0x005a1c9e in dbus_g_proxy_constructor (type=159204112, n_construct_properties=4, construct_properties=0x97d5f60) at dbus-gproxy.c:1311 #6 0x0015432a in IA__g_object_newv (object_type=159204112, n_parameters=4, parameters=0x97d7d60) at gobject.c:912 #7 0x00154ed9 in IA__g_object_new_valist (object_type=159204112, first_property_name=0x5ae10e "name", var_args=0xbf963974 "") at gobject.c:996 #8 0x00154fe0 in IA__g_object_new (object_type=159204112, first_property_name=0x5ae10e "name") at gobject.c:793 #9 0x005a0358 in dbus_g_proxy_new (connection=0x97d5bf4, name=0x649f3f "org.freedeskt op.DBus", path_name=0x649f54 "/org/freedesktop/DBus", interface_name=0x649f3f "org.freedesktop.DBus") at dbus-gproxy.c:1824 #10 0x00648247 in sniffer_dbus_register_service (pSniffer=0x97d42c8) at /home/heefre/software/dbus/wireshark/dbus/wireshark-dbus-sniffer.c:373 #11 0x00647ded in sniffer_dbus_init (pCaptureOptions=0x80553e0) at /home/heefre/software/dbus/wireshark/dbus/wireshark-dbus-sniffer.c:255 #12 0x0804fe29 in main (argc=2, argv=0xbf963b14) at /home/heefre/software/dbus/wireshark/dumpcap.c:381 If dbus_g_connection_unref is called 2 times, the second
dbus_g_connection_unref complains that the transport is still connected:
_dbus_warn ("The last reference on a connection was dropped without closing
the connection. This is a bug. See dbus_connection_unref() documentation for
details.\n");
From the list: It appears the problem is that in dbus-connection.c if a bus address has no "guid" property (as the default system bus address does not), we don't record the connection in the hash table of shared connections. Thus on shutdown in dbus-connection.c:shared_connections_shutdown we don't disconnect the connection, resulting in a leak. The fix is pretty simple; keep a list of shared connections that lack a guid and be sure each shared connection is either in the list or in the hash. Created attachment 12215 [details] [review] proposed patch Something like this? Seems to work. Thanks, some nitpicks on this one: - no_guid_connections I would rename to shared_connections_no_guid or something, to show the relationship to the shared_connections variable - the unlock/lock should not be inside connection_shutdown_helper because it keeps the locks from being visibly paired (i.e. all lock/unlock pairs should be visible in the same function). Instead just keep the unlock and lock where they were. - let's call connection_shutdown_helper something like close_connection_on_shutdown Created attachment 12242 [details] [review] fixed patch ok, here Should a guidless connection also be reusable? For example, if an application uses lot of libraries that all get their own system bus connections. without the guid, we would have no way to know the connections are the same. It seems that at least system bus connections are reused. I tested with multiple dbus_bus_get(DBUS_BUS_SYSTEM, &err) calls in a program. That's a different thing (the dbus_bus_get mechanism juse uses a global variable in dbus-bus.c, vs. shared connections in dbus-connection.c) This appears to have been fixed since dbus-1.1.20, back in 2008, by applying Kimmo's patch (commit 381c8548b2d). |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.