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

Issue 787144 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Jan 2
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , iOS
Pri: 1
Type: Bug



Sign in to add a comment

[Cronet] Bidirectional Stream API use of WrappedIOBuffer with app-provided buffer causes crash in QUIC memory slicer

Project Member Reported by mef@chromium.org, Nov 20 2017

Issue description

QUIC memory slicer takes ref-counted IOBuffer and uses it to avoid extra copies of data.

This doesn't work with Cronet implementation of BidirectionalStream::Write, which takes application-provided raw data pointer and wraps it using WrappedIOBuffer.

It works if net stack relinquishes ownership when onWriteCompleted is invoked, but it doesn't work with FLAGS_quic_reloadable_flag_quic_use_mem_slices=true because underlying code expects buffer to be retained by holding the additional reference, which results in crash like this:

libsystem_platform.dylib _platform_memmove : <line number missing>
Cronet net::QuicDataWriter::WriteBytes(void const*, unsigned long) : 151
Cronet net::QuicDataWriter::WriteBytes(void const*, unsigned long) : 151
Cronet net::QuicStreamSendBuffer::WriteStreamData(unsigned long long, unsigned long long, net::QuicDataWriter*) : 75
Cronet net::QuicFramer::AppendStreamFrame(net::QuicStreamFrame const&, bool, net::QuicDataWriter*) : 2025
Cronet net::QuicFramer::BuildDataPacket(net::QuicPacketHeader const&, std::__1::vector<net::QuicFrame, std::__1::allocator<net::QuicFrame> > const&, char*, unsigned long) : 384
Cronet net::QuicPacketCreator::SerializePacket(char*, unsigned long) : 429
Cronet net::QuicPacketCreator::Flush() : 260
Cronet net::QuicPacketGenerator::FinishBatchOperations() : 240
Cronet net::QuicConnection::ScopedPacketBundler::~ScopedPacketBundler() : 2276
Cronet net::QuicConnection::WriteAndBundleAcksIfNotBlocked() : 2269
Cronet base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) : 65
Cronet base::MessageLoop::RunTask(base::PendingTask*) : 394
Cronet base::MessageLoop::DoDelayedWork(base::TimeTicks*) : 406
Cronet base::MessagePumpCFRunLoopBase::RunWork() : 456
Cronet base::MessagePumpCFRunLoopBase::RunWorkSource(void*) : 428
CoreFoundation __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ : <line number missing>
CoreFoundation __CFRunLoopDoSource0 : <line number missing>
CoreFoundation __CFRunLoopDoSources0 : <line number missing>
CoreFoundation __CFRunLoopRun : <line number missing>
CoreFoundation CFRunLoopRunSpecific : <line number missing>
Foundation -[NSRunLoop(NSRunLoop) runMode:beforeDate:] : <line number missing>
Cronet base::MessagePumpNSRunLoop::DoRun(base::MessagePump::Delegate*) : 724
Cronet base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) : 179
 

Comment 1 by mef@chromium.org, Nov 20 2017

Cc: tmcarthur@google.com xunji...@chromium.org rch@chromium.org fayang@chromium.org
Labels: -OS-Linux OS-Android
The CL to turn FLAGS_quic_reloadable_flag_quic_use_mem_slices off: https://chromium-review.googlesource.com/c/chromium/src/+/780280 will need to be merged back into branch 3256 for cherry-picking in app build.
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 21 2017

Labels: merge-merged-3256
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b02bf92f4500900d413aaaa700fb5c4cdc7c51be

commit b02bf92f4500900d413aaaa700fb5c4cdc7c51be
Author: Fan Yang <fayang@chromium.org>
Date: Tue Nov 21 03:30:31 2017

Turn off FLAGS_quic_reloadable_flag_quic_use_mem_slices

Bug:  787144 
Change-Id: If067550312e59fd9aea4b498f14c6d2913c86a6d
Reviewed-on: https://chromium-review.googlesource.com/780280
Reviewed-by: Misha Efimov <mef@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Fan Yang <fayang@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#518027}(cherry picked from commit c5faf75c62f19fc45a365b941a6ea7a8d77f93a9)
Reviewed-on: https://chromium-review.googlesource.com/780463
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/branch-heads/3256@{#2}
Cr-Branched-From: b0a2dcae908c0df01ffa219ef4b48b5decc14f4c-refs/heads/master@{#513348}
[modify] https://crrev.com/b02bf92f4500900d413aaaa700fb5c4cdc7c51be/net/quic/core/quic_flags_list.h

Comment 3 by pkl@chromium.org, Nov 27 2017

Cc: mef@chromium.org
Owner: kapishnikov@chromium.org
Status: Assigned (was: Untriaged)
Owner: ----
Status: Untriaged (was: Assigned)
Owner: mef@chromium.org
Status: Assigned (was: Untriaged)
mef: can you find an appropriate owner for this bug?  Thanks!
Labels: M-70
This is important issue for QUIC performance and we should address it.
Status: Archived (was: Assigned)
We'll look at QUIC performance overall and re-visit this issue if it is a bottleneck.

Sign in to add a comment