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

Issue 910769 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Chrome
Pri: 1
Type: Bug



Sign in to add a comment

DCHECK() crash when unlocking w/ Smart Lock

Project Member Reported by khorimoto@chromium.org, Nov 30

Issue description

#0  0x00005d4bd0cf446a in logging::LogMessage::~LogMessage() () at ../../base/logging.cc:874                                                                                                        
#1  0x00005d4bcd12fcd0 in base::ObserverList<base::RunLoop::NestingObserver, false, true, base::internal::UncheckedObserverAdapter>::AddObserver(base::RunLoop::NestingObserver*) () at ../../base/observer_list.h:271
#2  0x00005d4bd37fe3b9 in proximity_auth::UnlockManagerImpl::UpdateLockScreen() () at ../../chromeos/components/proximity_auth/unlock_manager_impl.cc:495
#3  0x00005d4bd37ff00a in non-virtual thunk to proximity_auth::UnlockManagerImpl::OnProximityStateChanged() () at ../../chromeos/components/proximity_auth/unlock_manager_impl.cc:257
#4  0x00005d4bd38039ef in proximity_auth::ProximityMonitorImpl::CheckForProximityStateChange() () at ../../chromeos/components/proximity_auth/proximity_monitor_impl.cc:204
#5  0x00005d4bd38037cb in proximity_auth::ProximityMonitorImpl::OnGetRssi(base::Optional<int> const&) () at ../../build/cros_cache/chrome-sdk/tarballs/eve+11244.0.0+target_toolchain/usr/bin/../include/c++/v1/memory:2632
#6  0x00005d4bd380367b in proximity_auth::ProximityMonitorImpl::OnGetConnectionMetadata(mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>) () at ../../base/optional.h:92
#7  0x00005d4bd37ffdbf in void base::internal::InvokeHelper<true, void>::MakeItSo<void (proximity_auth::ProximityMonitorImpl::*)(mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>), base::WeakPtr<proximity_auth::ProximityMonitorImpl>, mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata> >(void (proximity_auth::ProximityMonitorImpl::*&&)(mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>), base::WeakPtr<proximity_auth::ProximityMonitorImpl>&&, mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>&&) () at ../../base/bind_internal.h:516
#8  0x00005d4bcec09e31 in chromeos::secure_channel::ClientChannelImpl::OnGetConnectionMetadata(base::OnceCallback<void (mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>)>, mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>) () at ../../base/callback.h:99
#9  0x00005d4bcec0a43c in void base::internal::FunctorTraits<void (chromeos::secure_channel::ClientChannelImpl::*)(base::OnceCallback<void (mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>)>, mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>), void>::Invoke<void (chromeos::secure_channel::ClientChannelImpl::*)(base::OnceCallback<void (mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>)>, mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>), base::WeakPtr<chromeos::secure_channel::ClientChannelImpl>, base::OnceCallback<void (mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>)>, mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata> >(void (chromeos::secure_channel::ClientChannelImpl::*)(base::OnceCallback<void (mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>)>, mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>),base::WeakPtr<chromeos::secure_channel::ClientChannelImpl>&&, base::OnceCallback<void (mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>)>&&, mojo::StructPtr<chromeos::secure_channel::mojom::ConnectionMetadata>&&) () at ../../base/bind_internal.h:516
#10 0x00005d4bcec0c727 in chromeos::secure_channel::mojom::Channel_GetConnectionMetadata_ForwardToCallback::Accept(mojo::Message*) () at ../../base/callback.h:99                                                                                                                                                                                         
#11 0x00005d4bd0e1fdb8 in mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message*) () at ../../mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:418
#12 0x00005d4bd0e31b56 in mojo::FilterChain::Accept(mojo::Message*) () at ../../mojo/public/cpp/bindings/lib/filter_chain.cc:40                                              
#13 0x00005d4bd0e211d2 in mojo::InterfaceEndpointClient::HandleIncomingMessage(mojo::Message*) () at ../../mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:306
#14 0x00005d4bd0e2733c in mojo::internal::MultiplexRouter::ProcessIncomingMessage(mojo::internal::MultiplexRouter::MessageWrapper*, mojo::internal::MultiplexRouter::ClientCallBehavior, base::SequencedTaskRunner*) () at ../../mojo/public/cpp/bindings/lib/multiplex_router.cc:869                                                              [0/1985]
#15 0x00005d4bd0e266ed in mojo::internal::MultiplexRouter::Accept(mojo::Message*) () at ../../mojo/public/cpp/bindings/lib/multiplex_router.cc:590
#16 0x00005d4bd0e31b56 in mojo::FilterChain::Accept(mojo::Message*) () at ../../mojo/public/cpp/bindings/lib/filter_chain.cc:40
#17 0x00005d4bd0e1d67c in mojo::Connector::ReadSingleMessage(unsigned int*) () at ../../mojo/public/cpp/bindings/lib/connector.cc:476
#18 0x00005d4bd0e1e1d4 in mojo::Connector::ReadAllAvailableMessages() () at ../../mojo/public/cpp/bindings/lib/connector.cc:505
#19 0x00005d4bd0e1e036 in mojo::Connector::OnHandleReadyInternal(unsigned int) () at ../../mojo/public/cpp/bindings/lib/connector.cc:387
#20 0x00005d4bcda52cb4 in mojo::SimpleWatcher::DiscardReadyState(base::RepeatingCallback<void (unsigned int)> const&, unsigned int, mojo::HandleSignalsState const&) () at ../../base/callback.h:129
#21 0x00005d4bd0e3ea63 in mojo::SimpleWatcher::OnHandleReady(int, unsigned int, mojo::HandleSignalsState const&) () at ../../base/callback.h:129
#22 0x00005d4bcdcd083e in void base::internal::Invoker<base::internal::BindState<void (content::FrameInputHandlerImpl::*)(int, int, std::__1::vector<ui::ImeTextSpan, std::__1::allocator<ui::ImeTextSpan> > const&), base::WeakPtr<content::FrameInputHandlerImpl>, int, int, std::__1::vector<ui::ImeTextSpan, std::__1::allocator<ui::ImeTextSpan> > >,void ()>::RunImpl<void (content::FrameInputHandlerImpl::*)(int, int, std::__1::vector<ui::ImeTextSpan, std::__1::allocator<ui::ImeTextSpan> > const&), std::__1::tuple<base::WeakPtr<content::FrameInputHandlerImpl>, int, int, std::__1::vector<ui::ImeTextSpan, std::__1::allocator<ui::ImeTextSpan> > >, 0ul, 1ul, 2ul, 3ul>(void(content::FrameInputHandlerImpl::*&&)(int, int, std::__1::vector<ui::ImeTextSpan, std::__1::allocator<ui::ImeTextSpan> > const&), std::__1::tuple<base::WeakPtr<content::FrameInputHandlerImpl>, int, int, std::__1::vector<ui::ImeTextSpan, std::__1::allocator<ui::ImeTextSpan> > >&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul>) ()    at ../../base/bind_internal.h:516                                                                                                                          
#23 0x00005d4bd0dbf457 in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) () at ../../base/callback.h:99
#24 0x00005d4bd0cfdcdf in base::MessageLoopImpl::RunTask(base::PendingTask*) () at ../../base/message_loop/message_loop_impl.cc:350
#25 0x00005d4bd0cfe392 in base::MessageLoopImpl::DoWork() () at ../../base/message_loop/message_loop_impl.cc:361                                                         
#26 0x00005d4bd0dbb529 in base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) () at ../../base/message_loop/message_pump_libevent.cc:210                                 
#27 0x00005d4bd0cfd7b5 in base::MessageLoopImpl::Run(bool) () at ../../base/message_loop/message_loop_impl.cc:302                                                                                        
#28 0x00005d4bd0d26f96 in base::RunLoop::Run() () at ../../base/run_loop.cc:102
#29 0x00005d4bd083cb95 in ChromeBrowserMainParts::MainMessageLoopRun(int*) () at ../../chrome/browser/chrome_browser_main.cc:1886
#30 0x00005d4bcdff4844 in content::BrowserMainLoop::RunMainMessageLoopParts() () at ../../content/browser/browser_main_loop.cc:999                       
#31 0x00005d4bcdff7253 in content::BrowserMainRunnerImpl::Run() () at ../../content/browser/browser_main_runner_impl.cc:165
#32 0x00005d4bcdff0faf in content::BrowserMain(content::MainFunctionParams const&) () at ../../content/browser/browser_main.cc:47
#33 0x00005d4bd082d4b0 in content::ContentMainRunnerImpl::RunServiceManager(content::MainFunctionParams&, bool) () at ../../content/app/content_main_runner_impl.cc:543
#34 0x00005d4bd082d356 in content::ContentMainRunnerImpl::Run(bool) () at ../../content/app/content_main_runner_impl.cc:866                                             
#35 0x00005d4bd0834c05 in service_manager::Main(service_manager::MainParams const&) () at ../../services/service_manager/embedder/main.cc:472                                         
#36 0x00005d4bd082b6f1 in content::ContentMain(content::ContentMainParams const&) () at ../../content/app/content_main.cc:19
#37 0x00005d4bccdb471f in ChromeMain () at ../../chrome/app/chrome_main.cc:102
#38 0x00007af7ad3bc736 in __libc_start_main (main=0x5d4bccdb4680 <main>, argc=31, argv=0x7fffb552fd58, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffb552fd48) at ../csu/libc-start.c:289
#39 0x00005d4bccdb4549 in _start ()
 
This crash is pretty bad if you have the "use Smart Lock for sign-in" feature enabled, since it essentially creates a crash loop at the sign-in screen.
Status: Started (was: Assigned)
Pretty sure I see the issue here. This AddObserver call [1] is being called over and over again.

Taking a look.

1) https://cs.chromium.org/chromium/src/chromeos/components/proximity_auth/unlock_manager_impl.cc?q=UnlockManagerImpl::UpdateLockScreen&sq=package:chromium&g=0&l=519
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 7

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

commit 2418c9546e5e7d0f2fabbb135c467e0df700f14e
Author: Ryan Hansberry <hansberry@chromium.org>
Date: Fri Dec 07 04:35:37 2018

Smart Lock: Only add observer to ProximityMonitor once.

ProximityMonitor::AddObserver was previously being called
multiple times. Its mirror RemoveObserver call was also
never called; this CL adds that call. Finally, the
proximity_monitor_ object was being created multiple times
for no reason; now it is created once.

Bug:  910769 
Change-Id: If1ab0216d364831cad2bf0d39516af31aa34cbd4
Reviewed-on: https://chromium-review.googlesource.com/c/1367084
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614589}
[modify] https://crrev.com/2418c9546e5e7d0f2fabbb135c467e0df700f14e/chromeos/components/proximity_auth/unlock_manager_impl.cc

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 10

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

commit 0bc9cb6e1f95ee2845a3d094174383effcdf99cb
Author: Ryan Hansberry <hansberry@chromium.org>
Date: Mon Dec 10 20:02:45 2018

Smart Lock: Move Observer handling from impl to base ProximityMonitor.

This makes test stubs and mocks use the common Observer logic. In a test
context, this will catch mistakes where Observers are added multiple times
or not removed (this would have caught  crbug.com/910769 ).

Bug:  910769 
Change-Id: I053d0605b5d4ec2f4b2da117b735a31c7447e804
Reviewed-on: https://chromium-review.googlesource.com/c/1368685
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615212}
[modify] https://crrev.com/0bc9cb6e1f95ee2845a3d094174383effcdf99cb/chromeos/components/proximity_auth/BUILD.gn
[add] https://crrev.com/0bc9cb6e1f95ee2845a3d094174383effcdf99cb/chromeos/components/proximity_auth/proximity_monitor.cc
[modify] https://crrev.com/0bc9cb6e1f95ee2845a3d094174383effcdf99cb/chromeos/components/proximity_auth/proximity_monitor.h
[modify] https://crrev.com/0bc9cb6e1f95ee2845a3d094174383effcdf99cb/chromeos/components/proximity_auth/proximity_monitor_impl.cc
[modify] https://crrev.com/0bc9cb6e1f95ee2845a3d094174383effcdf99cb/chromeos/components/proximity_auth/proximity_monitor_impl.h
[modify] https://crrev.com/0bc9cb6e1f95ee2845a3d094174383effcdf99cb/chromeos/components/proximity_auth/unlock_manager_impl_unittest.cc

Sign in to add a comment