Issue metadata
Sign in to add a comment
|
Regression : Chrome Crash is seen on entering 5/6/7 characters in 'Password' text box of any wi-Fi network |
||||||||||||||||||||||
Issue descriptionChrome Version: 60.0.3086.3/9517.1.0 dev-channel Candy,Daisy and Minnie OS: Chrome What steps will reproduce the problem? (1)Sign into User -> Click on Uber Tray (2)Now select Network option ->Try to click on any new network -> Enter 5/6/7 characters in 'Password' text box and click on 'Connect button (3)Now observe Chrome Crash (Please refer video) Expected: No Crash should be seen on entering 5/6/7 characters in 'Password' text box. It should try to connect to network and if Password is wrong 'Network Connection Error' Notification should be seen Actual: Instead Chrome crash is seen This is Regression issue as same is working fine in 59.0.3071.33/9460.20.0 dev-channel Candy @tbuckley : please confirm the Issue Below is the Crash id: 6eb10923b0000000 Stack Trace: Thread 4 CRASHED [SIGABRT @ 0x000003e8000008ad ] MAGIC SIGNATURE THREAD Stack Quality 100%Show frame trust levels 0x00007ef041165eb2 (libc-2.23.so -raise.c:54) raise 0x00007ef041167cd5 (libc-2.23.so -abort.c:89) abort 0x00005a87e5737064 (chrome+ 0x02c3e064) base::debug::BreakDebugger() 0x00005a87e574a42c (chrome+ 0x02c5142c) logging::LogMessage::~LogMessage() 0x00005a87e61a1be3 (chrome+ 0x036a8be3) dbus::Bus::OnConnectionDisconnectedFilter(DBusConnection*, DBusMessage*, void*) 0x00007ef04273e445 (libdbus-1.so.3.14.8 -dbus-connection.c:4709) dbus_connection_dispatch 0x00005a87e61a2898 (chrome+ 0x036a9898) dbus::Bus::ProcessAllIncomingDataIfAny() 0x00005a87e3a3fbb6 (chrome+ 0x00f46bb6) base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) 0x00005a87e3a323c2 (chrome+ 0x00f393c2) base::MessageLoop::RunTask(base::PendingTask*) 0x00005a87e3a335a8 (chrome+ 0x00f3a5a8) base::MessageLoop::DoWork() 0x00005a87e3a33dcb (chrome+ 0x00f3adcb) base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) 0x00005a87e576e1af (chrome+ 0x02c751af) base::RunLoop::Run() 0x00005a87e578e907 (chrome+ 0x02c95907) base::Thread::ThreadMain() 0x00005a87e5789d0c (chrome+ 0x02c90d0c) base::(anonymous namespace)::ThreadFunc(void*) 0x00007ef0424e7557 (libpthread-2.23.so -pthread_create.c:333) start_thread 0x00007ef04122908c (libc-2.23.so+ 0x000f708c) clone
,
May 5 2017
Though the stack trace is completely diff from Issue 718411 (fixed on 60.0.3086.3/9517.1.0) but based on somewhat similar manual repro scenario cc'ing mohsen@ for more inputs.
,
May 6 2017
,
May 6 2017
yeah, stack trace is different but it appears to be related to the change landed for issue 718411 since this crash was not in 3086.0 and the only change was the fix for above bug https://chromium.googlesource.com/chromium/src/+log/60.0.3086.0..60.0.3086.3?pretty=fuller&n=10000 mohsen@ can you evaluate?
,
May 8 2017
Removing Blink>Network which is about web APIs (XHR, WebSocket, etc.)
,
May 8 2017
That doesn't seem related to my UI change, rather looks like an issue in the networking stack. The reason you weren't seeing this crash before my change is, probably, that the crash I fixed didn't let the code reach this crash! Now the first crash is not happening, so you see the second one.
,
May 8 2017
I can reproduce this and strangely I am getting the same error as we have been seeing in 714547: "D-Bus connection was disconnected. Aborting." That is preceeded by a "SetProperties: Failed: Config.SetProperties Failed" error, which is expected. So I don't think this is related to any of the status area changes, but rather some change is causing certain types of DBus errors to trigger this CHECK: https://cs.chromium.org/chromium/src/dbus/bus.cc?q=bus.cc+package:%5Echromium$&dr&l=1189
,
May 8 2017
Yeah, this reminds me of issue 714547 too. Cc-ing some people who might have ideas about how other D-Bus errors could trigger a (bogus?) "Disconnected" error.
,
May 8 2017
I can reproduce this in chrome @3086.0 (#468250). So, either this is due to a cros change, or it is from an older chrome change and has not been reported.
,
May 8 2017
Also reproducible at chrome @3085.0 (#468217). Looking at where this is failing, it seems like it might be a symptom of a more serious dbus failure related to a chromeos change? Can we confirm where this crash was introduced? If the system tray crashes are getting in the way of the password dialog showing, it should be possible to trigger the dialog from the Settings UI instead.
,
May 8 2017
Well, the system tray crash ( issue 718411 ) should not happen before password dialog. They happen during the connecting animation. The password dialog closes the system tray, so no animation should be kicked off unless the system menu is re-opened while connecting. So, those crashes should not interfer with this one and last part of my comment in #6 is not valid!
,
May 8 2017
https://codereview.chromium.org/2808983002/ could be related? does this issue reproduce with the change reverted locally?
,
May 8 2017
That was the first thing I tried :) Didn't help.
,
May 8 2017
I've just commented here crbug.com/714547#c14 but I'm guessing that issue 714547 and this issue are a reappearance of issue 181655 . The stack trace at #c10 also included dbus::Bus::OnConnectionDisconnected() and dbus::Bus::OnConnectionDisconnectedFilter (). In short, empty object paths somehow caused Chrome getting "disconnected" signal. The fix at that time was to remove --disable-check from dbus's build config. https://chromium-review.googlesource.com/c/45545/5/sys-apps/dbus/dbus-1.6.8-r2.ebuild
,
May 8 2017
Ah, and it looks like we recently updated the dbus version to 1.10.12 : https://chromium-review.googlesource.com/c/461717/ Can we try removing that flag in the updated ebuild?
,
May 8 2017
Can we add PRESUBMIT check or something to prevent --disable-check (now --disable-checks?) from getting added again?
,
May 8 2017
Forgot to mention but passing empty object paths sounds like a bug. We should investigate where these come from and fix these.
,
May 8 2017
I don't have a particular reason for why I added back the --disable-checks flag during the dbus upgrade, probably just copied from gentoo upstream. The CL that removed it long ago was saying, "Let me remove this flag until fixing the actual root cause", so is the real bug still alive? Anyways, I don't have an objection of removing it again. Just that I might not be the best person to properly test it though - only work on chromium os occasionally.
,
May 9 2017
Apparently --disable-checks came from upstream ebuild. https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-apps/dbus/dbus-1.10.12.ebuild If the empty object path is the root cause and the sender is using libchrome's D-Bus library, we can add a CHECK in dbus::MessageWriter to prevent sending empty ObjectPath.
,
May 9 2017
Made a CL to remove --disable-checks. https://chromium-review.googlesource.com/c/499967/
,
May 10 2017
,
May 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/4afb4364174e7491eddd6cf757f35446acacd8c7 commit 4afb4364174e7491eddd6cf757f35446acacd8c7 Author: Ryo Hashimoto <hashimoto@google.com> Date: Thu May 11 03:58:08 2017 sys-apps/dbus: Remove --disable-check Building dbus with --disable-check results in mysterious behaviors when invalid values (e.g. empty object path) are sent on D-Bus. BUG= chromium:718814 TEST=emerge-$BOARD dbus Change-Id: I53572b0d238279afb9289a19d1c60e3f2e77bbf0 Reviewed-on: https://chromium-review.googlesource.com/499967 Commit-Ready: Ryo Hashimoto <hashimoto@chromium.org> Tested-by: Ryo Hashimoto <hashimoto@chromium.org> Reviewed-by: Satoru Takabayashi <satorux@google.com> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/4afb4364174e7491eddd6cf757f35446acacd8c7/sys-apps/dbus/dbus-1.10.12.ebuild [rename] https://crrev.com/4afb4364174e7491eddd6cf757f35446acacd8c7/sys-apps/dbus/dbus-1.10.12-r2.ebuild
,
May 11 2017
Now we are no longer using --disable-check when building dbus, so invalid values like empty object paths don't result in mysterious behavior like chrome crash. Steven, is it possible to locate the actual code which is sending invalid values with D-Bus (e.g. empty object paths, non-UTF8 strings)? Crash should be gone, but we may end up repeating this again when uprevving libdbus in future.
,
May 11 2017
I don't know a good way to identify that off the top of my head. I didn't see any indication that Chrome was sending bad info when I investigated this, so I wonder if it was caused by Shill sending empty object paths in a response when an error occurred (but I'm not at all certain that was the case). I think we should resolve this bug is fixed and maybe file a separate bug to investigate that?
,
May 12 2017
,
May 12 2017
OK, filed issue 721668 for investigation. We shouldn't see this crash no longer, closing as fixed.
,
May 18 2017
No crash observed on 9563.0.0, 60.0.3102.0 with repro steps in bug description.
,
Jun 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/005ac9903d0e8cd98281496f48bc1b3296b3769b commit 005ac9903d0e8cd98281496f48bc1b3296b3769b Author: Ryo Hashimoto <hashimoto@google.com> Date: Wed Jun 28 10:09:52 2017 sys-apps/dbus: Add a presubmit check against --disable-checks With --disable-checks, dbus daemon behaves in an unpredictable way. We should avoid adding this again when uprevving dbus. BUG= chromium:718814 TEST=repo upload Change-Id: Ic84be1c6428243e84b5f55685e601f50f573796c Reviewed-on: https://chromium-review.googlesource.com/505790 Commit-Ready: Ryo Hashimoto <hashimoto@chromium.org> Tested-by: Ryo Hashimoto <hashimoto@chromium.org> Reviewed-by: Ben Chan <benchan@chromium.org> [add] https://crrev.com/005ac9903d0e8cd98281496f48bc1b3296b3769b/sys-apps/dbus/presubmit_check.sh [modify] https://crrev.com/005ac9903d0e8cd98281496f48bc1b3296b3769b/PRESUBMIT.cfg |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by mmanchala@chromium.org
, May 5 2017