Crash in network code when opening Settings for secondary user |
||||||||
Issue descriptionGoogle Chrome 66.0.3336.3 (Official Build) canary (64-bit) Revision 0 Platform 10374.0.0 (Official Build) canary-channel caroline Firmware Version Google_Caroline.7820.356.0 Customization ID SAMSUNG-CAROLINE What steps will reproduce the problem? (1) Sign in user A (my corp account) (2) System tray > user > add user B (my personal account) (3) From second user's desktop, Chrome 3-dot menu, select "Settings" Browser crashes and restarts. Reports: go/crash/0b55549bf21e3abf go/crash/0587831c4aeb44b3 0x00007665951d1f6d (libdbus-1.so.3.14.8 -dbus-marshal-recursive.c:493 ) base_reader_next 0x00007665951d218d (libdbus-1.so.3.14.8 -dbus-marshal-recursive.c:508 ) struct_reader_next 0x00007665951d0e44 (libdbus-1.so.3.14.8 -dbus-marshal-recursive.c:1070 ) _dbus_type_reader_next 0x00007665951d2bf8 (libdbus-1.so.3.14.8 -dbus-marshal-recursive.c:2633 ) writer_write_reader_helper 0x00007665951d2af2 (libdbus-1.so.3.14.8 -dbus-marshal-recursive.c:2526 ) writer_write_reader_helper 0x00007665951d2af2 (libdbus-1.so.3.14.8 -dbus-marshal-recursive.c:2526 ) writer_write_reader_helper 0x00007665951d1dea (libdbus-1.so.3.14.8 -dbus-marshal-recursive.c:2698 ) _dbus_type_writer_write_reader_partial 0x00007665951d123e (libdbus-1.so.3.14.8 -dbus-marshal-recursive.c:1197 ) replacement_block_replace 0x00007665951d10b7 (libdbus-1.so.3.14.8 -dbus-marshal-recursive.c:1303 ) _dbus_type_reader_set_basic 0x00007665951d00ef (libdbus-1.so.3.14.8 -dbus-marshal-header.c:373 ) _dbus_header_set_field_basic 0x00007665951d73cf (libdbus-1.so.3.14.8 -dbus-message.c:2620 ) dbus_message_iter_close_container 0x00005eeea35bdc85 (chrome -message.cc:568 ) sk_sp<GrTextureProxy>::operator->() const 0x00005eeea35cbd06 (chrome -values_util.cc:258 ) dbus::AppendBasicTypeValueDataAsVariant(dbus::MessageWriter*, base::Value const&) 0x00005eeea35ae42e (chrome -shill_client_helper.cc:439 ) chromeos::(anonymous namespace)::AppendValueDataAsVariantInternal(dbus::MessageWriter*, base::Value const&, chromeos::(anonymous namespace)::DictionaryType) 0x00005eeea3562d2a (chrome -shill_device_client.cc:78 ) chromeos::(anonymous namespace)::ShillDeviceClientImpl::SetProperty(dbus::ObjectPath const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, base::Value const&, base::RepeatingCallback<void ()> const&, base::RepeatingCallback<void (std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)> const&) 0x00005eeea358463f (chrome -network_device_handler_impl.cc:117 ) chromeos::(anonymous namespace)::SetDevicePropertyInternal(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, base::Value const&, base::RepeatingCallback<void ()> const&, base::RepeatingCallback<void (std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::unique_ptr<base::DictionaryValue, std::__1::default_delete<base::DictionaryValue> >)> const&) 0x00005eeea35863c0 (chrome -network_device_handler_impl.cc:563 ) chromeos::NetworkDeviceHandlerImpl::ApplyMACAddressRandomizationToShill() 0x00005eeea34cad4d (chrome -network_state_handler.cc:1535 ) chromeos::NetworkStateHandler::NotifyDeviceListChanged() 0x00005eeea1e01f92 (chrome -tether_service.cc:394 ) TetherService::UpdateTetherTechnologyState() 0x00005eeea1e01c92 (chrome -tether_service.cc:357 ) TetherService::UpdateEnabledState() 0x00005eeea34cad4d (chrome -network_state_handler.cc:1535 ) chromeos::NetworkStateHandler::NotifyDeviceListChanged() 0x00005eeea1e01c92 (chrome -tether_service.cc:357 ) TetherService::UpdateEnabledState() I suspect my Pixel phone is somewhere within Bluetooth range. Steven, does this make any sense? Settings from the primary account works fine. Assigning to you just because it's reproducible and might be networking related, but feel free to re-route. Kyle, ideas? I see tether stuff in the stack. Maybe it's using the wrong ProfileKeyedService for something?
,
Feb 6 2018
Yep, this looks like a Tether-related issue. I checked out the crash reports you sent, and those stacks show what the issue really is: ... 0x00005823ddc8ed4d (chrome -network_state_handler.cc:1535 ) chromeos::NetworkStateHandler::NotifyDeviceListChanged() 0x00005823dc5c5f92 (chrome -tether_service.cc:394 ) TetherService::UpdateTetherTechnologyState() 0x00005823dc5c5c92 (chrome -tether_service.cc:357 ) TetherService::UpdateEnabledState() 0x00005823ddc8ed4d (chrome -network_state_handler.cc:1535 ) chromeos::NetworkStateHandler::NotifyDeviceListChanged() 0x00005823dc5c5c92 (chrome -tether_service.cc:357 ) TetherService::UpdateEnabledState() 0x00005823ddc8ed4d (chrome -network_state_handler.cc:1535 ) chromeos::NetworkStateHandler::NotifyDeviceListChanged() 0x00005823dc5c5f92 (chrome -tether_service.cc:394 ) TetherService::UpdateTetherTechnologyState() 0x00005823dc5c5c92 (chrome -tether_service.cc:357 ) TetherService::UpdateEnabledState() 0x00005823ddc8ed4d (chrome -network_state_handler.cc:1535 ) chromeos::NetworkStateHandler::NotifyDeviceListChanged() 0x00005823dc5c5c92 (chrome -tether_service.cc:357 ) TetherService::UpdateEnabledState() 0x00005823ddc8ed4d (chrome -network_state_handler.cc:1535 ) chromeos::NetworkStateHandler::NotifyDeviceListChanged() 0x00005823dc5c5f92 (chrome -tether_service.cc:394 ) TetherService::UpdateTetherTechnologyState() 0x00005823dc5c5c92 (chrome -tether_service.cc:357 ) TetherService::UpdateEnabledState() ... This is repeated a bunch of times in a row. Looks like there's some infinite recursion that eventually causes the system to run out of memory. James, have you reproduced this on 65, or is it just on 66?
,
Feb 6 2018
Dug into this crash a bit and figured out what was going on. Two TetherService instances are created: one for the primary user and one for the secondary user. In this case, the primary user has synced Tether hosts, but the secondary user does not. When the device list changes, each TetherService instance receives an update and tries to set the correct value for the TechnologyState: ENABLED for the primary user with synced Tether hosts, and UNAVAILBLE for the secondary user with no synced Tether hosts. This causes a loop where the state is changed back and forth between UNAVAILABLE and ENABLED until stack memory runs out. The solution here is just to disable TetherService for the secondary user. It should only be available to the primary user.
,
Feb 6 2018
It does not reproduce in M64 beta on samus. I don't have a device on M65 right now, sorry.
,
Feb 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/50c6e7244c49de95f7e263f2e69c746c11233ea1 commit 50c6e7244c49de95f7e263f2e69c746c11233ea1 Author: Kyle Horimoto <khorimoto@google.com> Date: Thu Feb 08 02:05:19 2018 [CrOS Tether] Do not instantiate TetherService for secondary users. This fixes a browser crash which can occur when multiple TetherService instances are created and interact incorrectly with each other. Bug: 809357 , 672263 Change-Id: I0cc38cc39999525456d20b93063c1f65eb01eb0b Reviewed-on: https://chromium-review.googlesource.com/905392 Commit-Queue: Kyle Horimoto <khorimoto@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Cr-Commit-Position: refs/heads/master@{#535252} [modify] https://crrev.com/50c6e7244c49de95f7e263f2e69c746c11233ea1/chrome/browser/chromeos/tether/tether_service.cc [modify] https://crrev.com/50c6e7244c49de95f7e263f2e69c746c11233ea1/chrome/browser/chromeos/tether/tether_service.h [modify] https://crrev.com/50c6e7244c49de95f7e263f2e69c746c11233ea1/chrome/browser/chromeos/tether/tether_service_unittest.cc
,
Feb 8 2018
,
Feb 9 2018
Your change meets the bar and is auto-approved for M65. Please go ahead and merge the CL to branch 3325 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 9 2018
,
Feb 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1aacbaed6467da4057d169ce1282864fb77a5d3b commit 1aacbaed6467da4057d169ce1282864fb77a5d3b Author: Kyle Horimoto <khorimoto@google.com> Date: Fri Feb 09 16:09:27 2018 [CrOS Tether] Do not instantiate TetherService for secondary users. This fixes a browser crash which can occur when multiple TetherService instances are created and interact incorrectly with each other. TBR=khorimoto@google.com (cherry picked from commit 50c6e7244c49de95f7e263f2e69c746c11233ea1) Bug: 809357 , 672263 Change-Id: I0cc38cc39999525456d20b93063c1f65eb01eb0b Reviewed-on: https://chromium-review.googlesource.com/905392 Commit-Queue: Kyle Horimoto <khorimoto@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#535252} Reviewed-on: https://chromium-review.googlesource.com/911593 Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Cr-Commit-Position: refs/branch-heads/3325@{#402} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/1aacbaed6467da4057d169ce1282864fb77a5d3b/chrome/browser/chromeos/tether/tether_service.cc [modify] https://crrev.com/1aacbaed6467da4057d169ce1282864fb77a5d3b/chrome/browser/chromeos/tether/tether_service.h [modify] https://crrev.com/1aacbaed6467da4057d169ce1282864fb77a5d3b/chrome/browser/chromeos/tether/tether_service_unittest.cc
,
Feb 12 2018
Issue 811421 has been merged into this issue.
,
Oct 10
Verified on Eve 11125.0.0. Re-open if you continue to see this issue. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by jamescook@chromium.org
, Feb 6 2018