[PATCH] Iteration wakeup

[Havoc Pennington]

Hi,

Fan Wu wrote:
> The reason I put wakeup_iteration at
> _dbus_connection_do_iteration_unlocked() because this function
> (_do_iteration_unlocked) is also called by
> _dbus_connection_flush_unlocked() and
> _dbus_connection_read_write_dispatch(). So put it there shall have
> more coverage than put it in
> _dbus_connection_send_preallocated_unlocked_no_update().
> 
> If you don't mind I'll put your full code into the revised fix and
> remove wakeup_mainloop from
> _dbus_connection_send_preallocated_unlocked_no_update.

That sounds good. My point is not about putting the code in one place or 
the other, just that I think the two wakeups should be in the same 
place, wherever that place is.

> The return value of TRUE means the wakeup mechanism has been executed
> while most likely FALSE means the DBUS thread is not blocking at the
> time the function is called. I don't have a good usage of the return
> value at the time. I suggest we leave it there as it is in case we do
> come up with something. I'll add more text warning prople about the
> multithread context.

I would think that any use of this return value would create a race 
condition, so maybe we should leave it out.

>>> +   retval = select (p->wakeup_fd+1, NULL, &wfds, NULL, &tv);
>>> +
>>> +   if (retval <= 0)
>>> +       return FALSE;
>>> +
>>> +   /*write one char*/
>>> +   if (write (p->wakeup_fd, "w", 1) >0 )
>>> +       return TRUE;
>> This is a race to try to avoid the blocking write this way with
>> select(); to avoid the race, you need to use a nonblocking write (i.e.
>> set the pipe fd nonblocking - I think there is example code already that
>> sets the connection socket nonblocking)
> 
> I see. So you are suggesting set the write end of the pipe
> nonblocking, and remove the select() in this function, and in this
> function just do a write(), correct?

Exactly, I think that will work.

> It's used at the beginning of _dbus_iteration_wakeup():
> 
>   > +   if (NULL == p || !p->is_polling)
>   > +       return FALSE;
> 
> So that if the DBUS thread is not doing polling, the wakeup will  not be run.

Is this a race? I would think it has to be unless is_polling is 
protected by a lock, but since we're doing wakeup() we can't have the io 
path lock at this point, right? So what lock protects is_polling?

There's no real need to avoid the wakeup I don't think, because we only 
need to do the wakeup if we fail to acquire the IO path lock. So usually 
we won't do the wakeup anyway.

> Do you want the testing to be self-contained (the testing code will
> spawn a daemon), or the test code assumes the availability of a
> daemon?

Either way. In the test directory, there's a directory called name-test, 
which should be called running-daemon-tests or something - it contains 
several tests that are against a running daemon.

Havoc