Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Improve eventloop control stability. Add eventloop and multithread tests #615

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

IanButterworth
Copy link
Collaborator

@IanButterworth IanButterworth commented Jan 17, 2022

Changes:

  • Removes the async eventloop stop. It was introducing too much instability.
    It was added to work around the MacOS bug where the window doesn't finish closing, which appears to be an upstream Gtk issue. https://gitlab.gnome.org/GNOME/gtk/-/issues/4648#note_1359210 . Perhaps we could leave this open for a few days and see if that gets fixed
  • Makes pause_eventloop wait for the eventloop to be paused
  • Fixes some bugs around stability
  • Add tests covering eventloop control and explicitly tests the multithreading issue, when guarded
  • Soft-deprecates Gtk.gtk_quit() -> Gtk.gtk_main_quit() as it matches the underlying function and gtk_main() better

@codecov
Copy link

codecov bot commented Jan 17, 2022

Codecov Report

Merging #615 (6735b15) into master (ca39439) will increase coverage by 0.64%.
The diff coverage is 91.42%.

❗ Current head 6735b15 differs from pull request most recent head f7e0109. Consider uploading reports for the commit f7e0109 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #615      +/-   ##
==========================================
+ Coverage   57.95%   58.59%   +0.64%     
==========================================
  Files          32       32              
  Lines        2721     2652      -69     
==========================================
- Hits         1577     1554      -23     
+ Misses       1144     1098      -46     
Impacted Files Coverage Δ
src/GLib/signals.jl 76.95% <ø> (-1.42%) ⬇️
src/displays.jl 52.71% <ø> (+19.56%) ⬆️
src/events.jl 45.91% <0.00%> (-1.03%) ⬇️
src/Gtk.jl 86.11% <92.85%> (-5.96%) ⬇️
src/GLib/GLib.jl 61.53% <100.00%> (-15.74%) ⬇️
src/base.jl 79.74% <100.00%> (ø)
src/builder.jl 85.71% <100.00%> (+85.71%) ⬆️
src/gtktypes.jl 33.33% <0.00%> (-40.01%) ⬇️
src/GLib/gtype.jl 63.13% <0.00%> (-12.90%) ⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca39439...f7e0109. Read the comment docs.

@IanButterworth IanButterworth changed the title Add wait_eventloop_stopping() and wait before recording state before a pause Add wait_eventloop_stopping(). Wait before recording state before a pause. Add multithread tests Jan 18, 2022
@IanButterworth IanButterworth marked this pull request as draft January 18, 2022 03:41
@IanButterworth
Copy link
Collaborator Author

IanButterworth commented Jan 18, 2022

Tests are passing but I'm still exploring this PR.

It would be nice to remove the async eventloop stop, but I'm still getting windows staying open and freezing on macos.


I'm exploring whether the macos thing is an upstream bug https://gitlab.gnome.org/GNOME/gtk/-/issues/4648

Update: it appears to be https://gitlab.gnome.org/GNOME/gtk/-/issues/4648#note_1358579

@IanButterworth IanButterworth force-pushed the ib/wait_to_report_if_stopping branch 2 times, most recently from f92e518 to 3c908a8 Compare January 19, 2022 01:08
@IanButterworth IanButterworth changed the title Add wait_eventloop_stopping(). Wait before recording state before a pause. Add multithread tests Improve eventloop control stability. Add eventloop and multithread tests Jan 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant