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

Issue 760619 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

storage::FileWriterDelegate is trying to read from a URLRequest it deleted.

Project Member Reported by mmenke@chromium.org, Aug 30 2017

Issue description

Chrome Version: 62.0.3192.0
OS: Windows (Probably all)

storage::FileWriterDelegate appears to be trying to read from a deleted URLRequest.  It's apparently deleting request_, and then its OnDataWritten method is invoked from an already pending write, at which point it tries to read again, resulting in a crash.

This is a low incidence crasher (267th most common on dev), but looks to be easy to fix, and infrequent crashers add up over time.

Link to instances of this crash on Windows:  https://crash.corp.google.com/browse?q=product.name%3D%27Chrome%27%20AND%20custom_data.ChromeCrashProto.ptype%3D%27browser%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27net%3A%3AURLRequest%3A%3AOnCallToDelegateComplete%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D&unnest=&stbtiq=&reportid=93173292186fa70a&index=4#0

I verified that request_ is nullptr in one minidump, didn't look at any others.  One trivial fix would be just to check if request_ is nullptr on write complete, though may be able to do something better (Delete the FileStreamWriter on canellation?  Not sure what happens with pending writes in that case).

Stack trace:

0x00007ffa94ebc29c	(chrome.dll -url_request.cc:1205 )	net::URLRequest::OnCallToDelegateComplete()
0x00007ffa94ebb1d8	(chrome.dll -url_request.cc:751 )	net::URLRequest::Read(net::IOBuffer *,int)
0x00007ffa95b0382f	(chrome.dll -file_writer_delegate.cc:121 )	storage::FileWriterDelegate::Read()
0x00007ffa95b03c25	(chrome.dll -file_writer_delegate.cc:170 )	storage::FileWriterDelegate::OnDataWritten(int)
0x00007ffa946b3877	(chrome.dll -bind_internal.h:339 )	base::internal::Invoker<base::internal::BindState<void ( media::cast::UdpTransport::*)(int),base::WeakPtr<media::cast::UdpTransport> >,void >::Run(base::internal::BindStateBase *,int &&)
0x00007ffa95b047fd	(chrome.dll -local_file_stream_writer.cc:241 )	storage::LocalFileStreamWriter::DidFlush(base::Callback<void ,1,1> const &,int)
0x00007ffa9475890f	(chrome.dll -bind_internal.h:339 )	base::internal::Invoker<base::internal::BindState<void ( content::BackgroundSyncManager::*)(base::Callback<void ,1,1> const &,content::ServiceWorkerStatusCode),base::WeakPtr<content::BackgroundSyncManager>,base::Callback<void ,1,1> >,void >::Run(base::internal::BindStateBase *,content::ServiceWorkerStatusCode &&)
0x00007ffa94fb4188	(chrome.dll -file_stream_context_win.cc:195 )	net::FileStream::Context::InvokeUserCallback()
0x00007ffa94fb40af	(chrome.dll -file_stream_context_win.cc:175 )	net::FileStream::Context::OnIOCompleted(base::MessagePumpForIO::IOContext *,unsigned long,unsigned long)
0x00007ffa94e97708	(chrome.dll -message_pump_win.cc:479 )	base::MessagePumpForIO::DoRunLoop()
0x00007ffa94e96c63	(chrome.dll -message_pump_win.cc:56 )	base::MessagePumpWin::Run(base::MessagePump::Delegate *)
0x00007ffa94e6ede3	(chrome.dll -run_loop.cc:111 )	base::RunLoop::Run()
0x00007ffa94782354	(chrome.dll -browser_thread_impl.cc:278 )	content::BrowserThreadImpl::IOThreadRun(base::RunLoop *)
 

Comment 1 by jsb...@chromium.org, Aug 30 2017

Cc: michaeln@chromium.org jsb...@chromium.org
i'll try invalidating the existing weakptrs to prevent odd reentrancy after canceling or erroring out.
Cc: -michaeln@chromium.org
Owner: michaeln@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 2 2017

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

commit 136394aa3612f8beb781ddc285e91b14d5f6b39b
Author: Michael Nordman <michaeln@google.com>
Date: Sat Sep 02 00:51:44 2017

Invalidate weakptrs to the FileWriterDelegate when cancelling.

Invalidate weakptrs to the FileWriterDelegate when the write operation
doesn't end gracefully. This prevents pending callbacks and potential
nullptr accesses (request_) after an error or cancellation has occurred.

Bug:  760619 
Change-Id: I0c54e609d1479fc9b7374ee5cb159d66a77cd7d4
Reviewed-on: https://chromium-review.googlesource.com/647384
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Commit-Queue: Michael Nordman <michaeln@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499360}
[modify] https://crrev.com/136394aa3612f8beb781ddc285e91b14d5f6b39b/storage/browser/fileapi/file_writer_delegate.cc

Status: Fixed (was: Started)

Sign in to add a comment