New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 835425 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Mac
Pri: 1
Type: Bug

Blocking:
issue 822433



Sign in to add a comment

Cronet Crash on QUIC DCHECK failure: !writer_->IsWriteBlocked()

Project Member Reported by zhongyi@chromium.org, Apr 20 2018

Issue description

See details in internal bug 77684625, 78282910

F0405 14:41:06.251542    8598 quic_connection.cc:1635] Check failed: !writer_->IsWriteBlocked() 
*** Check failure stack trace: ***
    @     0x7fc8bb32953e  base_logging::LogMessage::SendToLog()
    @     0x7fc8bb329d52  base_logging::LogMessage::Flush()
    @     0x7fc8bb32bc15  base_logging::LogMessageFatal::~LogMessageFatal()
    @     0x7fc8c8afa671  gfe_quic::QuicConnection::OnCanWrite()
    @     0x7fc8c8af88e3  gfe_quic::QuicConnection::WriteAndBundleAcksIfNotBlocked()
    @     0x7fc8c8af887d  gfe_quic::QuicConnection::MaybeSendInResponseToPacket()
    @     0x7fc8c8af9e59  gfe_quic::QuicConnection::ProcessUdpPacket()
    @     0x7fc8c8bf4b80  gfe_quic::QuicSession::ProcessUdpPacket()
    @     0x7fc8c8cad080  gfe_quic::QuartcSession::OnTransportReceived()
    @     0x7fc8c8ee16c2  quartc::QuartcThreadSafeSession::OnTransportReceivedAsync()::$_6::operator()()
    @     0x7fc8c8ee14fa  std::_Function_handler<>::_M_invoke()
    @     0x7fc8c8ee2f92  std::function<>::operator()()
    @     0x7fc8c8edfdab  quartc::(anonymous namespace)::ExecuteOnQuicThreadWithData()::$_10::operator()()
    @     0x7fc8c8edfbbd  std::_Function_handler<>::_M_invoke()
    @     0x7fc8c8fe8aad  std::function<>::operator()()
    @     0x7fc8c16ccf3c  util::functional::internal::FunctorCallback<>::Run()
    @     0x7fc8bbabfd46  ThreadPoolWorker::Run()
    @     0x7fc8bbab2616  Thread::ThreadBody()
    @     0x7fc8ba8aa4e8  start_thread
 
From ianswett@: 
"This must be a case of IsWriteBlocked changing from true to false between the call to it in WriteAndBundleAcksIfNotBlocked and OnCanWrite.

That seems like it could be a few things:
 1) The writer has unpredictable/unstable behavior.  If so, that's a bug with the writer.
 2) A packet was attempted to be written as a result of putting the ScopedPacketBundler on the stack, presumably because we're trying to write a large ack frame and it's either filling up the packet by itself or there's something else already queued and that's enough to fill up the rest."
Cc: ianswett@chromium.org mef@chromium.org
Components: Internals>Network>Library Internals>Network>QUIC
This change has been fixed by internal change list:  Change 191910257. And the flag protected fix in chromium is: 

Call QuicConnection::WriteIfNotBlocked instead of OnCanWrite() from
WriteAndBundleAcksIfNotBlocked to avoid writing another packet when the
writer is already blocked.

Protected by FLAGS_quic_reloadable_flag_quic_is_write_blocked.
Merge internal change: 191910257

Change-Id: Ic34b43a0ba198d0bbcf84f8f1b22918eb9fd44c9

It's landed in https://chromium-review.googlesource.com/c/chromium/src/+/1005563.
Labels: -Pri-3 OS-Android OS-Linux OS-Mac OS-Windows Pri-1
Labels: Merge-Request-67
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5f348f845f1baa924eede92bf883493162ae0205

commit 5f348f845f1baa924eede92bf883493162ae0205
Author: Zhongyi Shi <zhongyi@chromium.org>
Date: Fri Apr 20 21:15:27 2018

Flip quic_is_write_blocked to true to fix crash on cronet.

Details in internal bug 78282910.

Change-Id: Ie6a26f0f1ecbc937b83644e12ae9c87b6ef66410

Bug:  835425 
Change-Id: Ie6a26f0f1ecbc937b83644e12ae9c87b6ef66410
Reviewed-on: https://chromium-review.googlesource.com/1022253
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552471}
[modify] https://crrev.com/5f348f845f1baa924eede92bf883493162ae0205/net/quic/core/quic_flags_list.h

Project Member

Comment 6 by sheriffbot@chromium.org, Apr 21 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 7 by gov...@chromium.org, Apr 21 2018

Pls merge your change to M67 branch 3396 by 1:00 PM PT, Monday (04/23) so we can pick it up next M67 Dev/Beta release. Thank you.
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 23 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/93d538a65a131797fd4b2050277ab7ac46bf0c85

commit 93d538a65a131797fd4b2050277ab7ac46bf0c85
Author: Cherie Shi <zhongyi@chromium.org>
Date: Mon Apr 23 17:44:40 2018

Flip quic_is_write_blocked to true to fix crash on cronet.

Details in internal bug 78282910.

Change-Id: Ie6a26f0f1ecbc937b83644e12ae9c87b6ef66410

TBR=zhongyi@chromium.org

(cherry picked from commit 5f348f845f1baa924eede92bf883493162ae0205)

Bug:  835425 
Change-Id: Ie6a26f0f1ecbc937b83644e12ae9c87b6ef66410
Reviewed-on: https://chromium-review.googlesource.com/1022253
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#552471}
Reviewed-on: https://chromium-review.googlesource.com/1024281
Reviewed-by: Zhongyi Shi <zhongyi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#225}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/93d538a65a131797fd4b2050277ab7ac46bf0c85/net/quic/core/quic_flags_list.h

The original code to fix the crash in https://bugs.chromium.org/p/chromium/issues/detail?id=835425#c2 is already in M67 branch.

https://bugs.chromium.org/p/chromium/issues/detail?id=835425#c8 merged the flag flipping to M67. 

Everything looks good to me, just waiting the new cronet release to pick up those changes. 
Owner: mef@chromium.org
Handing off to Misha.

mef@: Misha, please help the cronet release to pick up the change, and close this bug once completed. Thanks!

Comment 11 by mef@chromium.org, Apr 26 2018

Blocking: 810883

Comment 12 by mef@chromium.org, Apr 26 2018

Blocking: 822433

Comment 13 by mef@chromium.org, Apr 26 2018

Commit 93d538a6... initially landed in 67.0.3396.17 so it is not in Android Dev channel (67.0.3396.11) build yet.
Blocking: -810883
Status: Fixed (was: Untriaged)
Closing this bug as Cronet latest release has picked up M68, the fix should be included. 

Sign in to add a comment