Re: client_side and comm_close

On Sat, 2008-04-19 at 11:52 +0200, Henrik Nordstrom wrote:
> fre 2008-04-18 klockan 08:25 -0600 skrev Alex Rousskov:
>
> > The changes needed on the client-side are pretty much unrelated to async
> > calls. Async calls just make the design problems more obvious and help
> > in solving them. Removing async calls from comm will fix a few bugs, but
> > will be a step backwards and will make ongoing code cleanup more
> > difficult.
>
> Here is a different proposal. Split async calls in two. Queued and
> immediate, with the event loops using immediate calls and everything
> else queued.

It is easy to implement two kinds of call scheduling, but I am not sure
I understand the merits of doing so. What do you mean by the "event
loops" in this context? Are you talking about eventAdd() queue? Or I/O
event (i.e., comm) callbacks? I am guessing it is the latter.

> The siplest way of doing so safely would be to make async
> calls immediate if not already within an async call, but adjusting the
> event loops is also a possibility. This would make the code behave a lot
> more like how it was before async, eleminating most races.

The immediate calls from comm were not really "safe" (because of the
recursive/reentrant behavior) so I am not sure what we would achieve
here. I hope answers to earlier questions would clarify the intent.
Again, I would like to understand and agree on the problem/goal before
debating specific solutions...

> The very few cases which may depend on comm_close being immediate should
> be possible to identify by cbdataInternalLock calls I think as the state
> object previously may disappear immediately otherwise. I can't see any
> in the client_side_* code today. But there very likely is assumptions
> that after comm_close has been called no further events will fire
> positively on that fd (only as ERR_COMM_CLOSING).

> I don't remember why comm_close was made async, but the more I think
> about it I think it's most likely wrong. comm_close SHOULD invalidate
> any pending comm operations on that fd, anything else just leads to
> nightmares.

Please clarify which of the following comm_close implementation would
solve the problems you want to solve in this context:

A) Immediately call all not-yet-dialed callbacks for the affected FD.
All relevant I/O callbacks are called with ERR_COMM_CLOSING flag set (if
we still have that). The closing callbacks, if any, are called last.

B) Asynchronously call all not-yet-dialed callbacks for the affected FD.
All relevant I/O callbacks are called with ERR_COMM_CLOSING flag set (if
we still have that). The closing callbacks, if any, are called last.

C) Other.

Implementation A would lead to reentrant job calls, which is the problem
we were trying to solve earlier, I think. Implementation B avoids
reentrant/recursive behavior. We can have both if B is the Right Thing
to do but A is what some old code required.

(I still would like to know what problem we are trying to solve though;
Is the problem you are trying to solve specific to comm_close?)

Thank you,

Alex.
Received on Tue Apr 22 2008 - 13:20:53 MDT