Issue metadata
Sign in to add a comment
|
DCHECK failure in PasswordStore code |
||||||||||||||||||||||
Issue description
For at least several weeks now, my locally build chromium often crashes with a DCHECK like this:
[14269:14302:0927/110037.053996:FATAL:bus.cc(805)] Check failed: origin_thread_id_ == base::PlatformThread::CurrentId() (14303 vs. 14302)
#0 base::debug::(anonymous namespace)::DebugBreak () at ../../base/debug/debugger_posix.cc:239
#1 0x00007ffff75778d8 in base::debug::BreakDebugger () at ../../base/debug/debugger_posix.cc:258
#2 0x00007ffff760c366 in logging::LogMessage::~LogMessage (this=0x7fffc778d060) at ../../base/logging.cc:791
#3 0x00007fffec9df990 in dbus::Bus::AssertOnOriginThread (this=0x2a921402f2e0) at ../../dbus/bus.cc:805
#4 0x00007fffec9dfad7 in dbus::Bus::AssertOnDBusThread (this=0x2a921402f2e0) at ../../dbus/bus.cc:814
#5 0x00007fffeca3c621 in dbus::ObjectProxy::CallMethodAndBlockWithErrorDetails (this=0x2a9214558ec0, method_call=0x7fffc778d6a0, timeout_ms=-1, error=0x7fffc778d528)
at ../../dbus/object_proxy.cc:79
#6 0x00007fffeca3d354 in dbus::ObjectProxy::CallMethodAndBlock (this=0x2a9214558ec0, method_call=0x7fffc778d6a0, timeout_ms=-1) at ../../dbus/object_proxy.cc:118
#7 0x000000010310f17d in KWalletDBus::Open (this=0x2a9214581938, wallet_name=..., app_name=..., handle_ptr=0x7fffc778da80)
at ../../components/os_crypt/kwallet_dbus.cc:144
#8 0x0000000102001784 in NativeBackendKWallet::WalletHandle (this=0x2a9214581920) at ../../chrome/browser/password_manager/native_backend_kwallet_x.cc:822
#9 0x00000001020055d0 in NativeBackendKWallet::GetAutofillableLogins (this=0x2a9214581920, forms=0x7fffc778dfc0)
at ../../chrome/browser/password_manager/native_backend_kwallet_x.cc:543
#10 0x0000000101f44b81 in PasswordStoreX::FillAutofillableLogins (this=0x2a9213c26e60, forms=0x7fffc778dfc0)
at ../../chrome/browser/password_manager/password_store_x.cc:221
#11 0x00000001048af0e4 in password_manager::PasswordStore::GetAutofillableLoginsWithAffiliationAndBrandingInformationImpl (this=0x2a9213c26e60, request=...)
at ../../components/password_manager/core/browser/password_store.cc:585
#12 0x00000001048c13de in base::internal::FunctorTraits<void (password_manager::PasswordStore::*)(std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >), void>::Invoke<scoped_refptr<password_manager::PasswordStore> const&, std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> > >(void (password_manager::PasswordStore::*)(std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >), scoped_refptr<password_manager::PasswordStore> const&, std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >&&) (method=
(void (password_manager::PasswordStore::*)(password_manager::PasswordStore * const, std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >)) 0x1048af000 <password_manager::PasswordStore::GetAutofillableLoginsWithAffiliationAndBrandingInformationImpl(std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >)>, receiver_ptr=..., args=<unknown type in /usr/local/google_ssd/mek/src/out/Default/chrome, CU 0x0, DIE 0x48ff6>) at ../../base/bind_internal.h:194
#13 0x00000001048c11ff in base::internal::InvokeHelper<false, void>::MakeItSo<void (password_manager::PasswordStore::* const&)(std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >), scoped_refptr<password_manager::PasswordStore> const&, std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> > >(void (password_manager::PasswordStore::* const&)(std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >), scoped_refptr<password_manager::PasswordStore> const&, std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >&&) (functor=
@0x2a9216e5f0c0: (void (password_manager::PasswordStore::*)(password_manager::PasswordStore * const, std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >)) 0x1048af000 <password_manager::PasswordStore::GetAutofillableLoginsWithAffiliationAndBrandingInformationImpl(std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >)>, args=<unknown type in /usr/local/google_ssd/mek/src/out/Default/chrome, CU 0x0, DIE 0x48f66>,
args=<unknown type in /usr/local/google_ssd/mek/src/out/Default/chrome, CU 0x0, DIE 0x48f66>) at ../../base/bind_internal.h:277
#14 0x00000001048c6786 in base::internal::Invoker<base::internal::BindState<void (password_manager::PasswordStore::*)(std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >), scoped_refptr<password_manager::PasswordStore>, base::internal::PassedWrapper<std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> > > >, void ()>::RunImpl<void (password_manager::PasswordStore::*)(std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >), std::__1::tuple<scoped_refptr<password_manager::PasswordStore>, base::internal::PassedWrapper<std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> > > >, 0ul, 1ul>(void (password_manager::Pass---Type <return> to continue, or q <return> to quit---
wordStore::*&&)(std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >), std::__1::tuple<scoped_refptr<password_manager::PasswordStore>, base::internal::PassedWrapper<std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> > > >&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>) (
functor=<unknown type in /usr/local/google_ssd/mek/src/out/Default/chrome, CU 0x0, DIE 0x30922>,
bound=<unknown type in /usr/local/google_ssd/mek/src/out/Default/chrome, CU 0x0, DIE 0x30930>) at ../../base/bind_internal.h:349
#15 0x00000001048c6679 in base::internal::Invoker<base::internal::BindState<void (password_manager::PasswordStore::*)(std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> >), scoped_refptr<password_manager::PasswordStore>, base::internal::PassedWrapper<std::__1::unique_ptr<password_manager::PasswordStore::GetLoginsRequest, std::__1::default_delete<password_manager::PasswordStore::GetLoginsRequest> > > >, void ()>::RunOnce(base::internal::BindStateBase*) (base=0x2a9216e5f0a0) at ../../base/bind_internal.h:318
#16 0x00007ffff7527ce1 in base::OnceCallback<void ()>::Run() && (this=0x7fffc778e500) at ../../base/callback.h:64
I wonder if some of the semi-recent migrations of tasks to different task runners is to blame here?
,
Sep 27 2017
,
Oct 2 2017
Exactly, the password manager switched to sequences while dbus didn't. hashimoto@, does dbus::Bus really need the same thread?
,
Oct 3 2017
+satorux As satorux@ said in https://codereview.chromium.org/2928513003/#msg6, due to the implementation in libdbus, we require a single thread for D-Bus just in case.
,
Oct 4 2017
+gab is there already a dedicated DBUS thread in chromium? if not, I guess we'll need to introduce one
,
Oct 10 2017
A dedicated thread exists on Chrome OS, and is managed by DBusThreadManager. IIRC, it does not exist on Linux desktop.
,
Oct 10 2017
post_task.h does vend SingleThreadTaskRunners as well as SequencedTaskRunners, sounds like you're forced to use a SingleThreadTaskRunner here.
,
Oct 16 2017
Issue 774583 has been merged into this issue.
,
Oct 16 2017
satorux@, hashimoto@ are you planning to relax the check in DBus code or we should use the single thread?
,
Oct 18 2017
No plan to relax the change because the check exists to prevent unsafe usage of libdbus.
,
Oct 18 2017
Thanks for the info, everyone. I'll change NativeBackendKWallet::background_task_runner_ to a SingleThreadTaskRunner.
,
Oct 18 2017
I'm not sure this will work. Doesn't dbus require all things dbus to be on one thread? What about adding a component "dbus_thread" that only has a getter for a lazy single thread task runner, and then everything (including chromeos' dbus thread manager) uses that?
,
Oct 18 2017
satorux@: Is there a bug tracking introducing the dedicated DBUS thread to Linux? NotificationPlatformBridgeLinuxImpl still just creates its own single thread runner, so even if I introduced a dedicated thread during fixing the issue for password manager code, the issue would remain open.
,
Oct 18 2017
,
Oct 18 2017
I missed Jochen's #12 when writing #13, so in case it feels confusing, here is a rephrasing: satorux@, does creating the "dbus_thread" component as Jochen suggests and make it provide the thread to be used by DBusThreadManager, NotificationPlatformBridgeLinuxImpl, password manager and OSCrypt sound good to you? I'm happy to do that provided we have a consensus.
,
Oct 19 2017
Issue 750996 has been merged into this issue.
,
Oct 23 2017
If apparently fixing this is so hard, could somebody at least disable the DCHECK for now? It's really annoying that I can't run chrome with DCHECKs enabled at all, since it is guaranteed to hit this DCHECK on startup.
,
Oct 24 2017
I'm sorry for the delay! Jochen gave me a base CL for #12 to build on and I planned to get to completing it yesterday. I did not get to it for another urgent issue, which seems mostly resolved now, so I hope to spend today fixing this DCHECK. I will aim at closing the day with either fixing it or disabling.
,
Oct 24 2017
Also, a workaround: if you force Chrome to avoid the KWallet password store, by passing --password-store=basic, the DCHECK should not fire for you.
,
Oct 24 2017
Update: https://chromium-review.googlesource.com/c/chromium/src/+/725289 in progress, once I see happy trybots, I'll send it out for review.
,
Oct 24 2017
I commented on the CL also, but I think that the existing model for Chrome OS is fine, we just need a simpler solution for Linux. Also, we already have src/dbus, I don't think that src/components is the right place for this.
,
Oct 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/440e0b2b638f1c3e892235277ff72e84ccf29ffb commit 440e0b2b638f1c3e892235277ff72e84ccf29ffb Author: Vaclav Brozek <vabr@chromium.org> Date: Thu Oct 26 06:36:31 2017 Introduce GetDBusTaskRunner, on Linux Code talking to DBus needs to run on the same thread across Chrome (see https://crbug.com/130984 ). This is controlled by DBusThreadManager on CrOS, but not on Linux. Therefore this CL introduces GetDBusTaskRunner() for vending a TaskRunner for a lazily created DBus thread, and also switches NativeBackendKWallet and NotificationPlatformBridgeLinuxImpl over to use it. (Credits: This CL contains substantial code from https://chromium-review.googlesource.com/c/chromium/src/+/727899.) Bug: 769381 Change-Id: I643fbdcf0efd5647fc3fc27106748ac2226c5e03 Reviewed-on: https://chromium-review.googlesource.com/725289 Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Christos Froussios <cfroussios@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: Vaclav Brozek <vabr@chromium.org> Cr-Commit-Position: refs/heads/master@{#511750} [modify] https://crrev.com/440e0b2b638f1c3e892235277ff72e84ccf29ffb/chrome/browser/BUILD.gn [add] https://crrev.com/440e0b2b638f1c3e892235277ff72e84ccf29ffb/chrome/browser/dbus/dbus_thread_linux.cc [add] https://crrev.com/440e0b2b638f1c3e892235277ff72e84ccf29ffb/chrome/browser/dbus/dbus_thread_linux.h [modify] https://crrev.com/440e0b2b638f1c3e892235277ff72e84ccf29ffb/chrome/browser/notifications/notification_platform_bridge_linux.cc [modify] https://crrev.com/440e0b2b638f1c3e892235277ff72e84ccf29ffb/chrome/browser/password_manager/native_backend_kwallet_x.cc [modify] https://crrev.com/440e0b2b638f1c3e892235277ff72e84ccf29ffb/chrome/browser/password_manager/native_backend_kwallet_x.h
,
Oct 26 2017
Thanks everybody for sharing their expertise for this. r511750 should fix the DCHECK. If it does not, please reopen this bug. Disclaimer: after this week I'm going to be OOO for about 10 days.
,
Oct 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c5edff99e343fb1d8e722ca1b8f6b1784528818f commit c5edff99e343fb1d8e722ca1b8f6b1784528818f Author: Vaclav Brozek <vabr@chromium.org> Date: Thu Oct 26 18:55:54 2017 Add //chrome/browser/dbus/OWNERS This OWNERS file was meant to be part of https://chromium-review.googlesource.com/c/chromium/src/+/725289 but for some reason got omitted from the upload. Bug: 769381 Change-Id: Ia58de3eb4107e86561f3498a7c25789ccae1dcc0 Reviewed-on: https://chromium-review.googlesource.com/737993 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Commit-Queue: Vaclav Brozek <vabr@chromium.org> Cr-Commit-Position: refs/heads/master@{#511900} [add] https://crrev.com/c5edff99e343fb1d8e722ca1b8f6b1784528818f/chrome/browser/dbus/OWNERS
,
Nov 29
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by mek@chromium.org
, Sep 27 2017