Re: IdleConnList problems

On 07/05/2011 09:18 AM, Amos Jeffries wrote:
> On 05/07/11 06:35, Tsantilas Christos wrote:
>> Hi all,
>> I am still having many problems with IdleConnList :-(
>>
>> Looks that the IdleConnList::pop() and the IdleConnList::findUseable()
>> methods return always NULL.
>> The reason is the fd_table[]::flags.read_pending flag which is always 0.
>> I can find only one place in the code where this flag set to 1/true, and
>> this is in ssl/support.cc file inside ssl_read_method function.
>>
>> The test:
>> if (!fd_table[theList_[i]->fd].flags.read_pending)
>> continue;
>> replaced an older check which used the comm_has_pending_read_callback
>> function which returned always false.
>>
>> What is the fde::flags::read_pending flag? What is supposed to do?
>
> The docs say it indicates whether the FD has a read event/call scheduled
> but not yet dialed.

It may be worth trying to implement read_pending flag.
I suppose we should set the flag in comm_read or even better in
Comm::Select functions.
But maybe it is more complex, because ssl code uses this flag (and looks
that currently is the only user)

> For idle conns that means the conn is about to close and is unsafe for use.
>
>>
>> Just removing the check for fd_table[].flags.read_pending in pop and
>> findUsable methods I am getting other assertions when trying to set a
>> comm-read handler on a connections retrieved from the connections
>> pool. ...
>
> According to the docs around the non-working
> comm_has_pending_read_callback() it was supposed to return true whenever
> a read handler callback was scheduled but not yet handled.
> That never actually worked.
>
> The quick fix is to drop that read_pending tests from idle conn
> handling. We are no more likely to hit the race conditions now with it
> broken as we are without it.

OK, for now I will try to just remove these tests ...

>
> On the side; read_pending and write_pending both seem to be special case
> flags, used for weird stuff unrelated to their documentation.
>
>
> Long term I think we should rename them to state what they actually do.
> Then implement wholly new read_pending/write_pending as a count of
> events pending a dial() to match the current documented meanings.

>
> Amos
Received on Tue Jul 05 2011 - 07:57:15 MDT