Re: IdleConnList problems

On 05/07/11 19:56, Tsantilas Christos wrote:
> 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.

Yes, I think the actual read-pending implementation it should be
incremented by the places doing FD_READ_METHOD() or
Comm::IoCallback::finish when they schedule the call. And decremented by
CommIoCbParams::dial() or syncWithComm() on reads.
  No other code should set it. But may read as needed.

> But maybe it is more complex, because ssl code uses this flag (and looks
> that currently is the only user)

Depends on what SSL_Pending(SSL_Ctx) means. I _think_ it means SSL needs
to do more I/O. It is currently being used to force both read() and
write() of the FD on every I/O cycle regardless of which is actually needed.
  That could do with some cleaning up, but it is not clear how to do so
without breaking SSL. For now I think a ssl_pending flag in place of the
current read_pending operations would be clearer.

>
>> 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 ...
>

You mentioned other asserts. Still happening and what are they?

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.14
   Beta testers wanted for 3.2.0.9