Phantom "Out of Memory" error

[Havoc Pennington]

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.

Anyway, then we could just keep the lock.

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.

Havoc