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
