On Sat, 2008-04-12 at 01:09 +0200, Henrik Nordstrom wrote: > fre 2008-04-11 klockan 10:52 -0600 skrev Alex Rousskov: > > > A comm_cancel_my_read wrapper will help mitigate risk (but not eliminate > > it). Note that once the comm_cancel_my_read is in place, the only > > mistake the programmer can make is to call Call::cancel instead of > > calling comm_cancel_my_read. The effect? Comm may poll the FD one extra > > time. We are not risking much here, right? > > right. > > but there is one more race here.. comm_read -> comm_read_cancel -> > another comm_read. What happens with data which the first had already > read from the fd? > > Fortunately comm_read_cancel isn't used in situations where it really > matters, but it's a general problem which is helpful to discuss now > rather than when it bites..
Are you talking about a situation where comm reads some data, places a "there is more data" call, and then the reader cancels that call before receiving it? In that case, the data will most likely not be used be the reader because it would not know to look for it. If the same receiver than changes its mind and asks comm to read again, then I believe the first data chunk will be skipped/ignored. In other words, there will be a problem. If we need to fix this, the answer is probably not in AsyncCalls but in how the I/O buffer is [not] passed to comm. IIRC, a similar problem exists in old code: there are places where we have to use two raw buffers because comm operates with raw memory pointers and does not update the metadata of a MemBuf correctly (or at all). If comm starts using MemBufs or similar, and the reader checks buffer state to see if there is data available before declaring an eof, then the above scenario will not cause any problems. In SMP context, using MemBufs (with locking and such) will not help though. One cannot pause a remote async comm and then resume it without losing data (unless comm itself starts buffering it). This is a topic of another discussion though. Does the current code cancel and resume reads? You said "comm_read_cancel isn't used in situations where it really matters" so I assume we do not do that, but this may be worth double checking. IIRC, some code does _pause_ and resumes reads (e.g., when the buffer gets full), but I hope it never cancels them; it just stops registering with comm for more reads until there is space. Thanks, Alex.
