dbus_g_proxy_end_call assumes that the passed-in call_id is not 0. However, dbus_g_proxy_begin_call can legitimately return 0, if the DBusConnection is disconnected but the Disconnect message has not yet been processed. This situation is handled gracefully in dbus_g_proxy_call (which was Bug #12675), but not in dbus_g_proxy_call_with_timeout (which seems to have been copied from dbus_g_proxy_call after Kimmo Hämäläinen wrote the patch for Bug #12675, but before Colin merged it). In any case, users of the asynchronous API can't be expected to handle this situation, given that it's never been documented. Relatedly, dbus_g_proxy_begin_call returns 0 on certain detectable programmer errors. One such error is failure to G_VALUE_COLLECT() one of the arguments - but on such errors, the DBUS_G_VALUE_ARRAY_COLLECT_ALL macro currently leaks and ignores the error. We should trap these errors and emit a critical warning. (The patch for DBUS_G_VALUE_ARRAY_COLLECT_ALL is based on patches by Kimmo Hämäläinen and Christian Dywan, for Maemo.)
Created attachment 48093 [details] [review] [PATCH 1/5] DBusGProxy: cope gracefully with call_id == 0 s well as happening on a programming error or OOM (which both provoke critical warnings), dbus_g_proxy_begin_call_internal() can legitimately fail if we got disconnected from D-Bus (so can't send any more), but are still working through our backlog of incoming messages (so we haven't got round to the Disconnected message yet, so the DBusGProxy hasn't emitted ::destroy). fd.o #12675 fixed dbus_g_proxy_call(), but dbus_g_proxy_call_with_timeout() and the asynchronous API still need similar treatment.
Created attachment 48094 [details] [review] [PATCH 2/5] dbus_g_proxy_call: simplify error handling now that call_id may be 0 There's no need to special-case it any more - we can just use dbus_g_proxy_end_call_internal and rely on it to cope. This reverts the fix for Bug #12675, since the commit before this one is more general.
Created attachment 48095 [details] [review] [PATCH 3/5] DBusGProxy: deal with errors in DBUS_G_VALUE_ARRAY_COLLECT_ALL G_VALUE_COLLECT can fail (admittedly, it's a programming error if it does). Previously, we leaked the resulting g_malloc'd string; now we emit a critical warning, free it, and abort the call. Based on patches from Kimmo Hämäläinen and Christian Dywan in Maemo's dbus-glib. Bug-NB: NB#86280 Bug-NB: NB#180486 (CID-34419 to CID-34424 inclusive)
Created attachment 48096 [details] [review] [PATCH 4/5] MyObject: register marshallers so individual tests don't have to These marshallers are actually dual-use: they're used to call certain methods on MyObject, and also to bind to its signals.
Created attachment 48097 [details] [review] [PATCH 5/5] Add a regression test for fd.o #38406
Review of attachment 48093 [details] [review]: no hat review+
Review of attachment 48094 [details] [review]: no hat review+
Review of attachment 48095 [details] [review]: no hat review+
Review of attachment 48096 [details] [review]: no hat review+
Review of attachment 48097 [details] [review]: no hat review+
This has been sitting here for weeks with no veto, so I'm going to consider Cosimo's review sufficient and merge this shortly.
Fixed in git for 0.96
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.