fre 2008-04-11 klockan 01:00 -0600 skrev Alex Rousskov: > Receiving or handling a call is not an instantaneous event though (and > never was). Consider this pseudo-code:
Yes. And at Call::dial calling foo.handleThissCall() I consider the call made. The fact that foo.handleThisCall() hasn't returned yet is mostly irrelevant. And in the current code base it's completely irrelevant as it's the only function executing at the moment, only becomes a concern the day we look into SMP. The window I consider troublesome at the moment is the time the call sits in the call queue and where other things may occur which would render the call unwanted. This is very similar to the recursive problem, but thankfully a lot easier to work with. > When one does not want to receive the call back. Ok. Some suggestions on how this design can be improved to ease things a bit, minimizing the risk for bad use. a) Comm needs to be redesigned to act as both caller and receiver for the legacy interfaces where it interfaces with function callback code and constructs internal Calls. Today it only acts as caller, making receiver cancellation of such calls unreliable. b) Call should be extended with a mechanism where the caller can ask to get a notification if the receier cancels the call before it's being made, eleminating the need for the receiver to both cancel the call and tell the caller that he is no longer interested. > In other words, only call recipients should cancel calls. What makes it > confusing is that the current code uses things like comm_read_cancel > which can be interpreted as the recipient canceling its callback but > might be called by non-recipients as well. The suggestions below try to > clear this mess. I do not view comm_read_cancel as having this dual personality. Should only be called by the same code which called comm_read. Any other use of comm_read_cancel is wrong. > Here is what I would suggest (specific application to comm_read is right > below these three general rules!): > > * Callbacks must be called: If Foo gets a call pointer as a callback, > Foo must make that call (possibly with a Foo-specific error indicator). > Whether that call will be dialed is irrelevant here. Yes, ignoring receiver initiated cancellations in this picture. > * Unwanted calls must be canceled: If Bar gives Foo a call pointer as a > callback, but may not be able to handle the call back later, then Bar > must keep a pointer to the same call and cancel that call when needed. > Whether that call will be scheduled (by Foo) is irrelevant here. Yes. And this is where comm currently fails.. > * Processed calls must be forgotten: If you scheduled, received, or > cancel the call, set its pointer to NULL. Foo above must forget the call > after scheduling it. Bar above must forget the call when it receives or > cancels it. These are just cleanup rules to avoid double-scheduling or > double-cancellation. Mistakes here are not fatal. Yes. > Let's apply these rules to your specific comm_read/comm_read_cancel > example (please read all three paragraphs below before complaining :-): > > Comm_read accepts and stores a callback pointer. Thus, it promises to > make that call no matter what. If things go out of hand with the file > descriptor, comm will make that call with a comm error flag set in the > parameters. Ok. > If the reader does not want to or cannot read any more, it cancels the > call. It needs to keep a pointer to the call if it might cancel the > call. If the same reader thinks that comm might still be reading from > that file descriptor, the reader also calls comm_read_cancel. This last > sentence is unrelated to calls; it just makes sure comm is not doing > something in vein. Comm_read_cancel cancels the read, not the callback! > Call:cancel cancels the call, not the read. I would like to get rid of the need to use comm_read_cancel here. Cancelling the call should be sufficient, and the caller should be able to derive from there when needed. > You may have noticed that in most cases, calling comm_read_cancel would > result in comm scheduling the call that will _not_ be dialed (or even > queued) because the reader has canceled it. This does not lead to bugs, > but wastes a few cycles. I am not so worried about no-op conditions. Those will need to be optimized later for performance reasons, but is not very important in this discussion. What I worry about is how to prove reliable cancellations, and minimizing the risk of developers doing things wrong. In this specific case receivers using the legacy callback function based versions of comm_read_cancel or commSetTimeout to cancel their requests expecting the request to be cancelled, but where the current code does not guarantee that because comm doesn't fulfill the requirements above. I also worry about having a design which requires more than one conscious action to be taken by the developer of a recipient to both cancel the call and cancel the operation which should make the call (mainly for performance reasons, but also helps getting rid of now unwanted chain effects).. > If and only if we are certain that comm_read_cancel is always called by > the reader, then we can eliminate that waste by giving comm_read_cancel > a pointer to the-call-pointer parameter. That adjusted comm_read_cancel > can both cancel/NULL the call and stop I/O monitoring. We can also > rename it to something like comm_cancel_my_read. That adjusted function > will sit partially in reader's space (canceling reader's call) and > partially in comm's space (canceling I/O). The line will be blurry, but > we will save a few cycles. Or we could get rid of comm_read_cancel entirely.. at least when dealing with AsyncCall based comm_read calls. > Now, let's see what comm_close do. Just like comm_read_cancel, it must > call all registered callbacks with ERR_CLOSING or some such. Again, raw > comm_close is not (or should not be) about callbacks; it is about I/O. > And again, if we want to optimize, we could add comm_close_my_io that > will cancel the callbacks first. Yes. And we had a similar race in comm_close earlier if you remember. > Does this answer your questions? Yes, but not yet in 100% agreement. Regards Henrik
