GNOME Bugzilla – Bug 355180
audit usage of CurrentTime and meta_display_get_current_time()
Last modified: 2020-11-06 20:08:19 UTC
Between bug 152000, bug 354422, and others I'm certain that I am forgetting, it's probably time to audit the code for usage of CurrentTime and meta_display_get_current_time() and remove them if at all possible. If any of them cannot be removed, a comment stating the exact reason why it can't be removed should be added. We should probably try to remove meta_display_get_current_time_roundtrip() where possible as well, though I'm suspecting that we only inserted those where we can't get a timestamp -- but we should at least document why.
Created attachment 72828 [details] [review] Partial audit Due to https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=206263, I thought I'd look at this a little sooner; this patch may help with that bug. Some notes: - This patch is incomplete; more usages of CurrentTime (and pinging for timestamps with meta_display_get_current_time_roundtrip()) could be removed. However, it appears that many cannot be removed, which kind of sucks. - I'd like to understand why display->current_time is cleared early for SelectionClear events. I have no idea why that would have been put there. - One or two of my warning messages may have been slightly over the top. ;) - There was another grab-related call with a CurrentTime timestamp in meta_display_begin_grab_op() that I missed in bug 354422. Honestly, I no longer even think I understand why the patch in bug 354422 actually prevents stuck grabs--but since it does, this patch may help too. - timestamp pinging could be pulled out of finish_minimize() and meta_window_show() and centralized in implement_showing()...with a bit of work - I think the use of CurrentTime in meta_window_update_keyboard_resize() looks particularly problematic; unfortunately, adding a timestamp parameter there has a much bigger side-effect than in most other places. It would mean adding a timestamp parameter to update_resize, which is called from several different timeouts where it'll take a bit more work to get the timestamp poked in. Nothing tricky at all, I just noticed how long the patch had already taken and lost a bit of interest.
Well, this patch shouldn't make things any worse and may actually fix some races; since others probably don't want to review such a huge patch, I've given myself a few days, and looked over it again to make sure it still looks sane. I've committed this patch, but as noted in comment 1 there is still more work to do. 2006-09-18 Elijah Newren <newren gmail com> Partial audit to fix timestamp usage. <snip> Remove usage of CurrentTime, meta_display_get_current_time() and meta_display_get_current_time_roundtrip() where possible, or document why it isn't possible, or at very least add a FIXME with some explanation of my laziness and what needs to be done.
bugzilla.gnome.org is being replaced by gitlab.gnome.org. We are closing all old bug reports in Bugzilla which have not seen updates for many years. If you can still reproduce this issue in a currently supported version of GNOME (currently that would be 3.38), then please feel free to report it at https://gitlab.gnome.org/GNOME/metacity/-/issues/ Thank you for reporting this issue and we are sorry it could not be fixed.