A RHEL customer is having some problems with a large number of simultaneous logins (even after the fixes in https://bugs.freedesktop.org/show_bug.cgi?id=48177 ) This bug adds a few patches to hopefully get things on track.
Created attachment 134665 [details] [review] lib: move g_object_ref call for clarity create_new_user creates an ActUser object and returns it to the caller. Before returning the object, it also adds the object to a list of "new users" to track when it fully loads. It's idiomatic to return the original ref to the caller of a constructor, but create_new_user instead gives the original reference to the new_users list. Their's no practical difference, but this commit moves the ref around for clarity.
Created attachment 134666 [details] [review] lib: leak fix If a new user is added by accounts service, we currently leak a reference to it. That leads to transient user objects living longer than they're supposed which uses memory and produces excess bus traffic.
Created attachment 134667 [details] [review] lib: another leak fix If a user gets removed from account service before it's fully loaded, then we neglect to drop its reference on the new_users lists. This commit fixes that.
Created attachment 134668 [details] [review] daemon: don't send spurious change signals when wtmp changes Right now, we unintentionally send out a changed signal for every tracked user anytime wtmp changes. This commit fixes that.
Created attachment 134669 [details] [review] lib: consolidate change notification if the daemon sends out multiple change notifications for a user because several properties changed in quick succession, try to batch them up, and only update info at the end.
Created attachment 134670 [details] [review] daemon: add new HasMultipleUsers and HasNoUsers properties Every gnome-shell instance wants to know if the system has multiple users or not, in order to know whether or not to show the 'Switch User' feature in the menu. accountsservice doesn't provide this information directly, though, so libaccountsservice instead requests a list of all users on the system and counts the provided list, filtering out system users. This is a lot of work for every gnome-shell instance to do, when it doesn't actually need the list of users at all. This adds a new property HasMultipleUsers which libaccountsservice can watch for instead. For good measure, this commit also adds a HasNoUsers boolean which can be used to know whether or not to start gnome-initial-setup.
Created attachment 134671 [details] [review] lib: use new has-multiple-users property from accountsservice This commit changes accountsservice to use the new has-multiple-users property provided by the daemon.
Created attachment 134672 [details] [review] lib: factor user loading functions into helpers Right now, we process ListCachedUser results open coded. This commit moves the processing to helper functions.
Created attachment 134673 [details] [review] lib: move accounts proxy creation to helper This commit factors out hte accounts proxy creation to a helper function.
Created attachment 134674 [details] [review] lib: retry connecting to accountsservice when loading users If we were unable to connect to accountsservice and we need to load users again, then try again.
Created attachment 134675 [details] [review] lib: simplify code dramatically ActUser is a wrapper over the accountsservice daemon's managed user objects. There's a nearly 1-to-1 correspondence between properties on the proxy to the daemon and properties on the ActUser object. This commit dramatically reduces the code, by leveraging the proxies properties directly, rather than duplicating the values on the ActUser object. At the same time, it drops manual GetAll() calls for synchronizing the proxies properties, since it's completely redundant with the work the proxy is doing under the hood anyway.
Created attachment 134676 [details] [review] lib: don't track user-added/user-removed until we get initial list There's no reason to process user-added and user-removed signals until we have our starting list. Those signals are supposed to be a delta off that list anyway.
Created attachment 134677 [details] [review] lib: only track users after act_user_manager_list_users At the moment, as soon as someone calls act_user_manager_get_default() we end up firing of an asynchronous call to get a list of all users. This is problematic since not all programs using accountsservice want a list of users. This commit changes the code to only get a list of users when the caller invokes act_user_manager_list_users. This does mean some calls that were async before are synchronous now, but user proxies were always obtained synchronously, and they're by far the slowest part of listing users, so I don't expect this introduce any noticeable blocking. Longer term, to fix the sync i/o bits, I think we should probably ditch libaccountsservice and just make accountsservice use ObjectManager interfaces over d-bus.
Is this still being worked on?
nope, this got pushed. i guess probably some sort of git-bz hiccup.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.