Phantom "Out of Memory" error

[Rafaël Carré]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Wed Jul 18 10:07:16 PDT 2007

Answering to
http://lists.freedesktop.org/archives/dbus/2007-July/008148.html
> Hi,
>
> keith preston wrote:
>> Would this actually work?   I think this has other issues.   Such as in
>> the Add_timeout case the user add_timeout function can fail and then we
>> would need to regain the lock and remove the timeout from the list.
>
> Hmm, yeah.
>
> A simplifying approach we could take is to say that these add_timeout
> etc. application functions are not allowed to call any dbus functions,
> in particular any methods on the connection. Before feeling comfortable
> with that I'd want to review existing implementations of the app
> callbacks here to be sure they aren't doing this.

I looked at dbus-glib source & dbus-python

In dbus-python, dbus_connection_set_timeout_functions() is not used

In dbus-glib: the add/remove/toggle functions use:

dbus_timeout_get_enabled()
dbus_timeout_set_data()
dbus_timeout_get_data()

And don't touch to the connection

> Anyway, then we could just keep the lock.

So that seems OK, I don't know what else would make use of these
functions except bindings

That would only require putting explicitely in
dbus_connection_set_timeout_functions() doc that the connection must not
be touched at all.

> Technically an ABI change but probably OK in practice.

>>     The simplest quick fix I can think of is to make
>>     timeout_list_add_timeout and friends return the closure thingy, then
>>     where the current code does timeouts=NULL and drops the lock, don't do
>>     the timeouts=NULL part since we won't need the timeout list, just the
>>     closure.
>>
>>
>> I think what I describe above is close to this, but I'm not completely sure.
>
> Yeah, the code you had is approximately what I meant (I'm being a little
> hand-wavy since I'd have to just write the code to be able to be precise ;-)
>
>> The only other way I see (which might be cleaner) is to pass a function
>> to lock and unlock the connection from protected_change_timeout to the
>> DBusTimeout{Add,Remove,Toggle}Functions.  This would make them
>> responsible for calling this unlock function before it call out to an
>> application callback and to lock it right after.
>
> Agreed, I had considered that and meant to mention it. It could be an
> easier approach.
>
> Obviously the existing code is hacky AND broken so I'm open to hacky and
> working patches, as well as clean and working patches. ;-) As long as
> the hacks don't leak into public API.

I propose to not unlock the connection when calling the
add/remove/toggle functions, that would just make the code a little less
broken ;)

But it wouldn't address the case where these functions fail, maybe the
functions could print an error message themselves, so we would have a
description of what happens AND the ugly "Out of Memory" message.

> Havoc

- --
Rafaël Carré
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHF0vBYWCeGMCv8Q8RAiFiAJ4+IJFsQO2/lHdd0yIdjLbNzCIwmQCg2LZg
IGmF8arU6qyfXCJMhVtzBuI=
=cpd6
-----END PGP SIGNATURE-----