[serial] Data ready to read immediately results in multiple read callbacks for one Read() call |
|||||
Issue descriptionBy fixing crbug.com/618911 I introduced a new issue. The problem arrises when Read() immediately has data and more data comes shortly after that data is read. The issues is, Read() always sets up a fd watcher *and* also tries immediate read. When the immediate read succeeds it will queue up a ReadCompleted(), then before that can be executed OnFileCanReadWithoutBlocking() will be called and another ReadCompleted() will be executed immediately. This causes a DCHECK failure when the first ReadCompleted() is finally executed because more than one read happened for one call to Read(). === Here is a simplified trace of the problem === SerialIoHandler::Read() ENTER SerialIoHandlerPosix::ReadImpl() SerialIoHandler::QueueReadCompleted() SerialIoHandler::Read() EXIT SerialIoHandlerPosix::OnFileCanReadWithoutBlocking() SerialIoHandler::ReadCompleted() SerialIoHandlerPosix::OnFileCanReadWithoutBlocking() SerialIoHandler::ReadCompleted() [0804/010643:FATAL:serial_io_handler.cc(227)] Check failed: IsReadPending(). ~
,
Aug 4 2016
,
Aug 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/577a8466eb739efbe3ba7887e64f9b230f62de0c commit 577a8466eb739efbe3ba7887e64f9b230f62de0c Author: aschulman <aschulman@chromium.org> Date: Thu Aug 04 22:29:58 2016 [serial] Only add a watcher if if the immeidate read attempt fails. BUG= 634278 Review-Url: https://codereview.chromium.org/2201893008 Cr-Commit-Position: refs/heads/master@{#409917} [modify] https://crrev.com/577a8466eb739efbe3ba7887e64f9b230f62de0c/device/serial/serial_io_handler_posix.cc [modify] https://crrev.com/577a8466eb739efbe3ba7887e64f9b230f62de0c/device/serial/serial_io_handler_posix.h
,
Aug 5 2016
This commit only fixed part of the problem. The 2nd part of the problem is that there is contention for the pending_read_buffer. An immediate read will queue up a ReadCompleted() call, but it won't cancel an existing watcher. This means that before the queued up ReadCompleted() can be called, the watcher may signal that the FD is ready, and another Read() will succeed overwrite (in a messy way) the pending_read_buffer, and call it's own non-queued up ReadCompleted().
,
Aug 5 2016
,
Aug 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/42b13d59919ca019e92b4decd0e7db5a636b1c32 commit 42b13d59919ca019e92b4decd0e7db5a636b1c32 Author: aschulman <aschulman@chromium.org> Date: Fri Aug 05 19:47:38 2016 Cancel read watcher if an immediate read succeeds. Otherwise the pending_read_buffer can be overwritten by the read watcher when it triggers before a queued up ReadCompleted() call. BUG= 634278 Review-Url: https://codereview.chromium.org/2212273002 Cr-Commit-Position: refs/heads/master@{#410148} [modify] https://crrev.com/42b13d59919ca019e92b4decd0e7db5a636b1c32/device/serial/serial_io_handler_posix.cc
,
Aug 9 2016
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by aschulman@chromium.org
, Aug 4 2016