Skip to content

Commit 786dba7

Browse files
donislawdevclaude
andcommitted
gui: a wait for quiet called off on its way does nothing when it arrives
The window's clock hands what it fires to the toolkit's queue (desktop.Later, time.AfterFunc then fyne.Do), and calling it off stops the timer, not a call already queued. If somebody typed in that gap, the quiet they broke still ran: it gave memory back twelve seconds later, in the middle of their work, and took the place of the new quiet's handle, so closing the window could no longer call that one off. tidy now counts the waits called off and each call checks it is still its own - the same as busy.epoch. The concurrency guard reported a declared file that runs nothing, but only among the files the walk reaches. A declared file that is gone kept its declaration and its place on the race job's list, green. The check is a function asked of a folder made in the test. The changelog said memory goes back after the last thing done in the window. It is the last setting changed or press of Preview or Generate - scrolling does not count - and it waits while work runs. Guards: TestAWaitCalledOffOnItsWayDoesNothingWhenItArrives (red on the old tidy.go, 2 waits and 2 releases), TestADeclarationWhoseFileIsGoneIsReported. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 9331b58 commit 786dba7

4 files changed

Lines changed: 107 additions & 11 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -496,10 +496,12 @@ because it turns other people's test suites red.
496496

497497
- **The window gives memory back once it has been left alone.** After a
498498
spell of work it used to keep about 200 MB for as long as it stood idle.
499-
About a minute and a half after the last thing done in it, the window now
500-
gives back what it no longer uses - 215 MB down to 129 MB in one
501-
measurement. Nothing on the screen moves when it does. A minimised window
502-
gives back less, because the toolkit only lets go while it draws.
499+
About a minute and a half after the last setting changed or the last press
500+
of Preview or Generate, the window now gives back what it no longer uses -
501+
215 MB down to 129 MB in one measurement. Scrolling does not count, and
502+
while Preview or Generate is running it waits. Nothing on the screen moves
503+
when it does. A minimised window gives back less, because the toolkit only
504+
lets go while it draws.
503505

504506
- **Changing a setting of a preset is about four times faster.** With
505507
`upload-validation`, changing one of its settings held the window for

‎internal/guard/concurrency_test.go‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,11 @@ func TestConcurrencyStaysWhereItWasPutOnPurpose(t *testing.T) {
140140
// how a decision gets undone quietly: the window gives memory back on a
141141
// goroutine BECAUSE a call on its own thread took 2491 ms once, and taking
142142
// the go statement away would leave the file declared and every guard
143-
// green. A file the build leaves out on this machine is not asked.
143+
// green. A file the build leaves out on this machine is not asked. A file
144+
// that is gone is, because the walk above never reaches it and a deleted
145+
// file would otherwise keep its declaration and its place on the race
146+
// detector's list - an outside review of the pull request named it.
147+
idle = append(idle, declaredWithoutAFile(repoRoot(t), mayBeConcurrent)...)
144148
if len(idle) > 0 {
145149
sort.Strings(idle)
146150
t.Errorf("declared as concurrent and running nothing beside anything:\n %s\n\n"+
@@ -158,6 +162,35 @@ func TestConcurrencyStaysWhereItWasPutOnPurpose(t *testing.T) {
158162
}
159163
}
160164

165+
// declaredWithoutAFile is every declared path with no file under root, each
166+
// with what the system said about it.
167+
func declaredWithoutAFile(root string, declared map[string]string) []string {
168+
var gone []string
169+
for rel := range declared {
170+
if _, err := os.Stat(filepath.Join(root, filepath.FromSlash(rel))); err != nil {
171+
gone = append(gone, rel+" ("+err.Error()+")")
172+
}
173+
}
174+
return gone
175+
}
176+
177+
// A declaration whose file is gone is reported, and one whose file is there
178+
// is not - asked of a folder made here, because every file the tree declares
179+
// exists and a check that stopped looking would stay green on it.
180+
func TestADeclarationWhoseFileIsGoneIsReported(t *testing.T) {
181+
root := t.TempDir()
182+
if err := os.MkdirAll(filepath.Join(root, "a"), 0o755); err != nil {
183+
t.Fatal(err)
184+
}
185+
if err := os.WriteFile(filepath.Join(root, "a", "here.go"), []byte("package a\n"), 0o644); err != nil {
186+
t.Fatal(err)
187+
}
188+
got := declaredWithoutAFile(root, map[string]string{"a/here.go": "", "a/gone.go": ""})
189+
if len(got) != 1 || !strings.HasPrefix(got[0], "a/gone.go ") {
190+
t.Errorf("declared a/here.go, which exists, and a/gone.go, which does not - reported %q, expected a/gone.go alone", got)
191+
}
192+
}
193+
161194
// concurrencyIn is every place in one file that runs something beside the
162195
// code around it, as "line what".
163196
//

‎internal/guard/tidy_test.go‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,49 @@ func TestSomethingDoneDuringTheWaitStartsTheQuietOver(t *testing.T) {
8888
}
8989
}
9090

91+
// A wait called off on its way does nothing when it arrives.
92+
//
93+
// Calling the clock off is not enough, and an outside review of the pull
94+
// request named why: the real window's clock hands what it fires to the
95+
// toolkit's queue (desktop.Later, time.AfterFunc then fyne.Do), and a call
96+
// already queued still runs. If somebody types in that gap, the quiet they
97+
// broke arrived anyway - it gave memory back twelve seconds later, in the
98+
// middle of their work, and took the place of the new quiet's handle, so
99+
// closing the window could no longer call that one off. The same gap as the
100+
// busy face's (TestAFaceAskedForByEarlierWorkNeverDressesLaterWork).
101+
//
102+
// Played out with the held clock: the first quiet is kept aside, a second key
103+
// calls it off, and then it is fired as if the queue had just got round to it.
104+
func TestAWaitCalledOffOnItsWayDoesNothingWhenItArrives(t *testing.T) {
105+
host := newFakeHost(t)
106+
window.Open(host)
107+
host.quiet = &quietClock{}
108+
content := tabNamed(t, host.content, text.TabOneTarget())
109+
110+
fill(t, content, text.FieldSeed(), "5")
111+
first := host.quiet.waiting()
112+
if len(first) != 1 {
113+
t.Fatalf("typing into a box left %d wait(s) for quiet, expected one to keep aside", len(first))
114+
}
115+
fill(t, content, text.FieldSeed(), "6")
116+
if !first[0].calledOff {
117+
t.Fatal("a second key did not call the first quiet off, so there is no call on its way to ask about")
118+
}
119+
120+
first[0].then() // the first quiet arriving from the queue after all
121+
if got := len(host.quiet.waiting()); got != 1 {
122+
t.Errorf("a quiet called off on its way arrived and %d wait(s) are left, expected the one quiet of the second key", got)
123+
}
124+
host.quiet.fireAll() // the second key's quiet
125+
if host.releases != 0 {
126+
t.Error("memory was given back after a quiet somebody had broken, in the middle of their work")
127+
}
128+
host.quiet.fireAll()
129+
if host.releases != 1 {
130+
t.Errorf("after the second key's quiet and its frame memory was given back %d time(s), expected once", host.releases)
131+
}
132+
}
133+
91134
// No memory goes back while work owns the window - the run is what is using it.
92135
func TestNoMemoryIsGivenBackWhileWorkIsGoing(t *testing.T) {
93136
host, content, hold := heldScreen(t)

‎internal/gui/window/tidy.go‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,12 @@ type tidy struct {
7878
// newTidy.
7979
later later
8080
callOff func()
81+
// wait counts the waits called off, so that one called off on its way
82+
// does nothing when it arrives. Calling the clock off is not enough: the
83+
// real window's clock hands what it fires to the toolkit's queue, and a
84+
// call already queued when somebody types still runs. The same as
85+
// busy.epoch, and an outside review of the pull request named it.
86+
wait int
8187
}
8288

8389
// newTidy builds the wait for one window.
@@ -100,32 +106,44 @@ func newTidy(h Host, busy func() bool) *tidy {
100106
// touch is told that something happened, and starts the quiet over.
101107
func (t *tidy) touch() {
102108
t.Stop()
103-
t.callOff = t.later(tidyAfterQuiet, t.frame)
109+
t.after(tidyAfterQuiet, t.frame)
104110
}
105111

106-
// Stop calls off whatever is being waited for. Closing the window stops it,
107-
// along with every screen.
112+
// Stop calls off whatever is being waited for, a call already on its way
113+
// included. Closing the window stops it, along with every screen.
108114
func (t *tidy) Stop() {
115+
t.wait++
109116
if t.callOff != nil {
110117
t.callOff()
111118
t.callOff = nil
112119
}
113120
}
114121

122+
// after asks the clock for then, which arrives only if nothing called the
123+
// wait off in the meantime.
124+
func (t *tidy) after(d time.Duration, then func()) {
125+
mine := t.wait
126+
t.callOff = t.later(d, func() {
127+
if t.wait != mine {
128+
return
129+
}
130+
t.callOff = nil
131+
then()
132+
})
133+
}
134+
115135
// frame asks the toolkit to draw, so that it lets go of what has expired. Even
116136
// under a run, which costs one frame and nothing else - whether to give memory
117137
// back is asked once, in release.
118138
func (t *tidy) frame() {
119-
t.callOff = nil
120139
if c := t.host.Canvas(); c != nil && c.Content() != nil {
121140
c.Refresh(c.Content())
122141
}
123-
t.callOff = t.later(tidyAfterFrame, t.release)
142+
t.after(tidyAfterFrame, t.release)
124143
}
125144

126145
// release gives the memory back, beside the window.
127146
func (t *tidy) release() {
128-
t.callOff = nil
129147
if t.busy() {
130148
t.touch()
131149
return

0 commit comments

Comments
 (0)