Adding second account when the first one is in auth error state crashes Chrome |
||||||
Issue descriptionChrome Version: HEAD OS: Goobuntu What steps will reproduce the problem? (1) Sign in with Account A (2) Change password of Account A on another computer (3) Go to gmail.com (4) On the page that asks you to re-enter your password, choose "Add different account" (5) Sign in with a different account What is the expected result? No crashed Chrome What happens instead? Crashed Chrome Log output: [245278:245278:0320/135442.931046:VERBOSE1:mutable_profile_oauth2_token_service_delegate.cc(309)] MutablePO2TS::RefreshTokenIsAvailable [245278:245278:0320/135442.931114:VERBOSE1:mutable_profile_oauth2_token_service_delegate.cc(309)] MutablePO2TS::RefreshTokenIsAvailable Received signal 11 SEGV_MAPERR 000000000028 #0 0x7f6bd5c7f6ad base::debug::StackTrace::StackTrace() #1 0x7f6bd5c7db9c base::debug::StackTrace::StackTrace() #2 0x7f6bd5c7f075 base::debug::(anonymous namespace)::StackDumpSignalHandler() #3 0x7f6bd61e20c0 <unknown> #4 0x7f6bd6166ca7 std::__1::basic_string<>::basic_string() #5 0x5563eb6d8ce0 [245323:245323:0320/135442.935178:VERBOSE1:gpu_channel.cc(416)] sending message @0x4eb69c7a660 on channel @0x4eb642d0020 with type 524476 [245323:245323:0320/135442.935761:VERBOSE1:gpu_channel.cc(496)] received message @0x4eb64ceb4c0 on channel @0x4eb642d0020 with type 524452 [245323:245323:0320/135442.936216:VERBOSE1:gpu_channel.cc(496)] received message @0x4eb64b95060 on channel @0x4eb642d0020 with type 524513 [245323:245323:0320/135442.936599:VERBOSE1:gpu_channel.cc(496)] received message @0x4eb67527bc0 on channel @0x4eb642d0020 with type 524452 [245323:245323:0320/135442.937990:VERBOSE1:gpu_channel.cc(496)] received message @0x4eb681fa140 on channel @0x4eb642d0020 with type 524452 [245323:245323:0320/135442.939216:VERBOSE1:gpu_channel.cc(496)] received message @0x4eb6930dae0 on channel @0x4eb642d0020 with type 524513 [245323:245323:0320/135442.939665:VERBOSE1:gpu_channel.cc(416)] sending message @0x4eb69c7b8e0 on channel @0x4eb642d0020 with type 524481 [245323:245323:0320/135442.941194:VERBOSE1:gpu_channel.cc(496)] received message @0x4eb680d53e0 on channel @0x4eb642d0020 with type 524452 [245323:245323:0320/135442.942399:VERBOSE1:gpu_channel.cc(496)] received message @0x4eb68f575a0 on channel @0x4eb642d0020 with type 524513 [245323:245323:0320/135442.943908:VERBOSE1:gpu_channel.cc(496)] received message @0x4eb66af44c0 on channel @0x4eb642d0020 with type 524452 [1:7:0320/135442.944843:VERBOSE1:node.cc(429)] Merging local ports B73E62F5FBCE9048.64D7E856169EAF5D@5C1C81F09D81EF5E.14F53DDA38F0937B and 9D26685F37292F11.DF61351677AE3049@5C1C81F09D81EF5E.14F53DDA38F0937B MutableProfileOAuth2TokenServiceDelegate::GetRefreshToken() #6 0x5563eb6d9452 [245323:245323:0320/135442.946076:VERBOSE1:gpu_channel.cc(496)] received message @0x4eb684e93e0 on channel @0x4eb642d0020 with type 524513 [1:7:0320/135442.948945:VERBOSE1:socket_dispatcher.cc(74)] P2PSocketDispatcher::OnFilterAdded() [245278:251072:0320/135442.950417:VERBOSE1:platform_thread_linux.cc(164)] Failed to set nice value of thread (253761) to -8: Permission denied (13) [1:7:0320/135442.954341:VERBOSE1:persistent_histogram_allocator.cc(997)] 144 histograms were created before persistence was enabled. [1:7:0320/135442.954699:VERBOSE1:persistent_histogram_allocator.cc(557)] Creating the results-histogram inside persistent memory can cause future allocations to crash if that memory is ever released (for testing). MutableProfileOAuth2TokenServiceDelegate::RefreshTokenIsAvailable() #7 0x5563e9729888 OAuth2TokenService::RefreshTokenIsAvailable() #8 0x5563ec42f887 [1:1:0320/135442.943194:ERROR:PaintPropertyTreePrinter.cpp(204)] Transform tree: David reconstructed this to: #0 0x7f6bd5c7f6ad base::debug::StackTrace::StackTrace() #1 0x7f6bd5c7db9c base::debug::StackTrace::StackTrace() #2 0x7f6bd5c7f075 base::debug::(anonymous namespace)::StackDumpSignalHandler() #3 0x7f6bd61e20c0 <unknown> #4 0x7f6bd6166ca7 std::__1::basic_string<>::basic_string() #5 MutableProfileOAuth2TokenServiceDelegate::GetRefreshToken() #6 MutableProfileOAuth2TokenServiceDelegate::RefreshTokenIsAvailable() #7 0x5563e9729888 OAuth2TokenService::RefreshTokenIsAvailable()
,
Mar 20 2018
Reconstructed last part of stack: #8 0x5563ec42f887 AboutSigninInternals::SigninStatus::ToValue() #9 0x5563ec42ae32 AboutSigninInternals::NotifyObservers() #10 0x5563ec432f45 AboutSigninInternals::OnErrorChanged() #11 0x5563ec48a6b7 SigninErrorController::AuthStatusChanged() #12 0x5563ec4893a1 SigninErrorController::AddProvider() #13 0x5563eb6d7945 MutableProfileOAuth2TokenServiceDelegate::AccountStatus::AccountStatus() #14 0x5563eb6deb51 MutableProfileOAuth2TokenServiceDelegate::UpdateCredentialsInMemory() #15 0x5563eb6dfd92 MutableProfileOAuth2TokenServiceDelegate::UpdateCredentials() #16 0x5563ec485759 ProfileOAuth2TokenService::UpdateCredentials() #17 0x5563eba28593 DiceResponseHandler::OnTokenExchangeSuccess() #18 0x5563eba283e4 DiceResponseHandler::DiceTokenFetcher::OnClientOAuthSuccess() #19 0x5563e96f76b8 GaiaAuthFetcher::OnOAuth2TokenPairFetched() #20 0x5563e96f9f6e GaiaAuthFetcher::DispatchFetchedRequest() #21 0x5563e96f99ae GaiaAuthFetcher::OnURLFetchComplete() #22 0x7f6bd3c2e889 net::URLFetcherCore::InformDelegateFetchIsComplete() #23 0x7f6bd3c2e769 net::URLFetcherCore::OnCompletedURLRequest() #24 0x7f6bd3c31aea _ZN4base8internal13FunctorTraitsIMN3net14URLFetcherCoreEFvNS_9TimeDeltaEEvE6InvokeIRK13scoped_refptrIS3_EJRKS4_EEEvS6_OT_DpOT0_ #25 0x7f6bd3c31a3f _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN3net14URLFetcherCoreEFvNS_9TimeDeltaEEJRK13scoped_refptrIS5_ERKS6_EEEvOT_DpOT0_ #26 0x7f6bd3c319cd _ZN4base8internal7InvokerINS0_9BindStateIMN3net14URLFetcherCoreEFvNS_9TimeDeltaEEJ13scoped_refptrIS4_ES5_EEEFvvEE7RunImplIRKS7_RKNSt3__15tupleIJS9_S5_EEEJLm0ELm1EEEEvOT_OT0_NSG_16integer_sequenceImJXspT1_EEEE #27 0x7f6bd3c318dc _ZN4base8internal7InvokerINS0_9BindStateIMN3net14URLFetcherCoreEFvNS_9TimeDeltaEEJ13scoped_refptrIS4_ES5_EEEFvvEE3RunEPNS0_13BindStateBaseE #28 0x7f6bd5c2e1b1 _ZNO4base12OnceCallbackIFvvEE3RunEv #29 0x7f6bd5c835ef base::debug::TaskAnnotator::RunTask() #30 0x7f6bd5d223c9 base::internal::IncomingTaskQueue::RunTask() #31 0x7f6bd5d2b5bb base::MessageLoop::RunTask() #32 0x7f6bd5d2b858 base::MessageLoop::DeferOrRunPendingTask() #33 0x7f6bd5d2bb68 base::MessageLoop::DoWork() #34 0x7f6bd5d2e73c base::MessagePumpGlib::HandleDispatch() #35 0x7f6bd5d2ee91 base::(anonymous namespace)::WorkSourceDispatch() #36 0x7f6bb9ab67f7 g_main_context_dispatch #37 0x7f6bb9ab6a60 <unknown> #38 0x7f6bb9ab6b0c g_main_context_iteration #39 0x7f6bd5d2e82f base::MessagePumpGlib::Run() #40 0x7f6bd5d2ad7c base::MessageLoop::Run() #41 0x7f6bd5de137d base::RunLoop::Run() #42 0x5563eb1c807e ChromeBrowserMainParts::MainMessageLoopRun() #43 0x7f6bcf90d871 [245323:245323:0320/135443.152601:VERBOSE1:gpu_channel.cc(496)] received message @0x4eb66af4920 on channel @0x4eb6b45baa0 with type 524452
,
Mar 20 2018
I looked at the code a bit, and here are some notes: - AboutSigninInternals is in the stack, probably open signin-internals to repro - we are in the constructor of AccountStatus (in #13). This is going to be added to the map. - in #5 we query the map, while being in the constructor of AccountStatus. I don't see why it would crash, but this is a strange state. I expect that the account is not yet in the map, and thus GetRefreshToken() will not find it, which is probably unexpected.
,
Mar 20 2018
Moving to M67, since we're moving Dice to M67.
,
Mar 21 2018
I could repro, here is the clean stack trace: Received signal 11 SEGV_MAPERR 000000000028 #0 0x7fc1e365847d base::debug::StackTrace::StackTrace() #1 0x7fc1e3656a3c base::debug::StackTrace::StackTrace() #2 0x7fc1e3657ed4 base::debug::(anonymous namespace)::StackDumpSignalHandler() #3 0x7fc1e3bb70c0 <unknown> #4 0x7fc1e3b3b637 std::__1::basic_string<>::basic_string() #5 0x556fbe04c710 MutableProfileOAuth2TokenServiceDelegate::GetRefreshToken() #6 0x556fbe04ce8f MutableProfileOAuth2TokenServiceDelegate::RefreshTokenIsAvailable() #7 0x556fbc10beb8 OAuth2TokenService::RefreshTokenIsAvailable() #8 0x556fbed7c420 AboutSigninInternals::SigninStatus::ToValue() #9 0x556fbed779b5 AboutSigninInternals::NotifyObservers() #10 0x556fbed7fae5 AboutSigninInternals::OnErrorChanged() #11 0x556fbedd6ae8 SigninErrorController::AuthStatusChanged() #12 0x556fbedd57c1 SigninErrorController::AddProvider() #13 0x556fbe04b395 MutableProfileOAuth2TokenServiceDelegate::AccountStatus::AccountStatus() #14 0x556fbe0526f1 MutableProfileOAuth2TokenServiceDelegate::UpdateCredentialsInMemory() #15 0x556fbe053a7b MutableProfileOAuth2TokenServiceDelegate::UpdateCredentials() #16 0x556fbedd1b79 ProfileOAuth2TokenService::UpdateCredentials() #17 0x556fbe3915d3 DiceResponseHandler::OnTokenExchangeSuccess() #18 0x556fbe391424 DiceResponseHandler::DiceTokenFetcher::OnClientOAuthSuccess()
,
Mar 21 2018
The line that is crashing is: return iter->second->refresh_token(); where |second| is an AccountStatus. It crashes, because second is null here. It is null because we are in the process of adding the account to the token service (see frame #14), and at this point we created the entry in the map, but did not populate it yet (the AccountStatus that we want to put in the map is still being constructed, frame #13). So when we do GetRefreshToken() on frame #5, we find the entry in the map, but it is null, causing the crash.
,
Mar 21 2018
Adding a Hotlist label to make it clear that this is necessary for our "Dice MVP"
,
Mar 22 2018
Fix in progress: https://chromium-review.googlesource.com/c/chromium/src/+/973370
,
Mar 22 2018
,
Mar 22 2018
I don't think this is related to Dice, the bug was likely existing before. Updating title and labels.
,
Mar 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/246f8fd23cbe2168d71bd7eee8b3927df27280ae commit 246f8fd23cbe2168d71bd7eee8b3927df27280ae Author: David Roger <droger@chromium.org> Date: Thu Mar 22 12:05:37 2018 Fix crash in about://signin-internals when adding account in auth error The crash was caused by SigninErrorController being called before the AccountStatus was added to the map. Then when the about-internals page tried to handle the error, it would try to get the account, but it would crash because it was not completely added in the AccountStatusMap. This CL also converts link_ptr to unique_ptr as a cleanup. Bug: 823707 Change-Id: I0019da48de6e281d812538633a90f519f6b24d3a Reviewed-on: https://chromium-review.googlesource.com/973370 Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Commit-Queue: David Roger <droger@chromium.org> Cr-Commit-Position: refs/heads/master@{#545044} [modify] https://crrev.com/246f8fd23cbe2168d71bd7eee8b3927df27280ae/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc [modify] https://crrev.com/246f8fd23cbe2168d71bd7eee8b3927df27280ae/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.h [modify] https://crrev.com/246f8fd23cbe2168d71bd7eee8b3927df27280ae/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc
,
Mar 22 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by droger@chromium.org
, Mar 20 2018Owner: droger@chromium.org
Status: Assigned (was: Untriaged)