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

Issue 605294 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 597790



Sign in to add a comment

Blimp Client crashes immediately after engine disconnect.

Project Member Reported by khushals...@chromium.org, Apr 20 2016

Issue description

If I trigger an engine crash, the Blimp client crashes immediately after it with a segmentation fault.

The crash seems to be in the BlimpConnection. Here is the symbolized stack trace if that helps:

Stack frame #00 pc 00a129f0  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine __c11_atomic_load<int> at /usr/local/google/code/clankium/src/out/an/../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/atomic:653 (discriminator 1)
Stack frame #01 pc 00a136a5  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine base::internal::WeakReferenceOwner::HasRefs() const at /usr/local/google/code/clankium/src/out/an/../../base/memory/weak_ptr.h:130 (discriminator 4)
Stack frame #02 pc 000b537f  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine base::SupportsWeakPtr<base::ObserverListBase<blimp::ConnectionErrorObserver> >::AsWeakPtr() at /usr/local/google/code/clankium/src/out/an/../../base/memory/weak_ptr.h:321
Stack frame #03 pc 000b5827  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine OnWritePacketComplete at /usr/local/google/code/clankium/src/out/an/../../blimp/net/blimp_connection.cc:96
Stack frame #04 pc 000b4c2d  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine Run<int> at /usr/local/google/code/clankium/src/out/an/../../base/bind_internal.h:181 (discriminator 5)
Stack frame #05 pc 000b6513  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine base::Callback<void (int), (base::internal::CopyMode)1>::Run(int) const at /usr/local/google/code/clankium/src/out/an/../../base/callback.h:397 (discriminator 2)
Stack frame #06 pc 00a00d7f  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine base::Callback<void (), (base::internal::CopyMode)1>::Run() const at /usr/local/google/code/clankium/src/out/an/../../base/callback.h:397 (discriminator 1)
Stack frame #07 pc 00a143fd  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine base::MessageLoop::RunTask(base::PendingTask const&) at /usr/local/google/code/clankium/src/out/an/../../base/message_loop/message_loop.cc:479
Stack frame #08 pc 00a14a0b  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine base::MessageLoop::DeferOrRunPendingTask(base::PendingTask const&) at /usr/local/google/code/clankium/src/out/an/../../base/message_loop/message_loop.cc:488
Stack frame #09 pc 00a14ae7  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine base::MessageLoop::DoWork() at /usr/local/google/code/clankium/src/out/an/../../base/message_loop/message_loop.cc:600
Stack frame #10 pc 00a16a31  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) at /usr/local/google/code/clankium/src/out/an/../../base/message_loop/message_pump_libevent.cc:230
Stack frame #11 pc 00a15b5f  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine base::MessageLoop::RunHandler() at /usr/local/google/code/clankium/src/out/an/../../base/message_loop/message_loop.cc:443 (discriminator 1)
Stack frame #12 pc 00a29f85  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine base::RunLoop::Run() at /usr/local/google/code/clankium/src/out/an/../../base/run_loop.cc:35
Stack frame #13 pc 00a14161  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine base::MessageLoop::Run() at /usr/local/google/code/clankium/src/out/an/../../base/message_loop/message_loop.cc:295
Stack frame #14 pc 00a43423  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine base::Thread::ThreadMain() at /usr/local/google/code/clankium/src/out/an/../../base/threading/thread.cc:254
Stack frame #15 pc 00a3fe77  /data/data/org.chromium.blimp/incremental-install-files/lib/libblimp_client_android.so: Routine ThreadFunc at /usr/local/google/code/clankium/src/out/an/../../base/threading/platform_thread_posix.cc:70
Stack frame #16 pc 0003f45f  /system/lib/libc.so (_ZL15__pthread_startPv+30)
Stack frame #17 pc 00019b43  /system/lib/libc.so (__start_thread+6)

 
Is the error deterministic? What steps did you take on the client? In what manner did you crash the engine? Was the engine running on your workstation?
Its easily reproducible, just start the engine on your local workstation and kill the process.
Error is due to colliding error handling methods - the callback AND THEN the error observer are triggered.

Callback is invoked with error code; the connection is deleted; error observers are invoked (w/ bare ptr, not weakptr!); observer method in a *deleted* connection references freed contents of connection.

Comment 4 by w...@chromium.org, Apr 21 2016

<archer>Bare pointers? Do you _want_ crashes? Because that's how you get
crashes!</archer>
Status: Started (was: Assigned)

Comment 6 by w...@chromium.org, May 23 2016

Labels: M-53 OS-Android

Comment 7 by w...@chromium.org, May 23 2016

Cc: haibinlu@chromium.org
Any progress on this, Kevin?
Labels: Blimp-M53-Proj-Scope
[Bulk edit]

Setting tracking label Blimp-M53-Proj-Scope.  This label is for scope tracking purposes only and should not be added / removed from any bugs, even if we add additional bugs to M-53 scope, or remove this bug from M-53 scope.
Project Member

Comment 9 by bugdroid1@chromium.org, May 26 2016

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

commit 793dac0c76b5f43baea388f71bdb5ff0929be915
Author: kmarshall <kmarshall@chromium.org>
Date: Thu May 26 22:03:06 2016

Use ConnectionErrorObserver, not callbacks, for error handling.

The current implementation invokes error observers and a callback
for read and write errors. The responsibility for cleanup and
object deletion was therefore ambiguous, making the callback
and observer clash in certain conditions.

This CL discards the callback entirely in favor of exclusive use of
the error observer for signaling errors.

R=wez@chromium.org
CC=khushalsagar@chromium.org
BUG= 605294 

Review-Url: https://codereview.chromium.org/1909143002
Cr-Commit-Position: refs/heads/master@{#396294}

[modify] https://crrev.com/793dac0c76b5f43baea388f71bdb5ff0929be915/blimp/net/blimp_connection.cc
[modify] https://crrev.com/793dac0c76b5f43baea388f71bdb5ff0929be915/blimp/net/blimp_connection_unittest.cc
[modify] https://crrev.com/793dac0c76b5f43baea388f71bdb5ff0929be915/blimp/net/blimp_message_output_buffer.cc

Status: Fixed (was: Started)
Labels: Archive-Blimp

Sign in to add a comment