Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "runtime" && test == "TestWindowsStackMemory"

--- FAIL: TestWindowsStackMemory (0.03s)
    crash_test.go:58: C:\Users\gopher\AppData\Local\Temp\1\go-build1812702813\testprog.exe StackMemory (27.351ms): ok
    syscall_windows_test.go:675: expected < 102400 bytes of memory per thread, got 103424

— watchflakes

• The test computes the average assuming there are 100 threads. If there are actually 101 threads (the test doesn't consider sysmon...), then the average comes out to exactly 102400 bytes.

The test was suggested by @aclements at #22439 (comment) . You should be able to adjust the test to allow for a couple of extra threads to be started by runtime without TestWindowsStackMemory noticing. But we want to keep the test to prevent regressions like this one #22439.

Alex.

In triage, I think we agree this is a test issue. @alexbrainman up for sending a patch? If not one of us will get around to it (especially if it happens again).

On CL 473275: https://storage.googleapis.com/go-build-log/67b9e0cb/windows-386-2016_1815f49f.log

Change https://go.dev/cl/473415 mentions this issue: runtime: allow for 5 more threads in TestWindowsStackMemory*

@alexbrainman up for sending a patch?

Here is my CL

https://go-review.googlesource.com/c/go/+/473415

Alex

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "runtime" && test == "TestWindowsStackMemory"

--- FAIL: TestWindowsStackMemory (0.03s)
    crash_test.go:58: C:\Users\gopher\AppData\Local\Temp\1\go-build3536125218\testprog.exe StackMemory (27.3413ms): ok
    syscall_windows_test.go:675: expected < 102400 bytes of memory per thread, got 102440

— watchflakes

2023-06-21 16:11 windows-386-2016 go@03063101 runtime.TestWindowsStackMemory

I can see that this failure happened on release-branch.go1.20 release branch - on commit

commit 03063101a2b40e78a44bdc1da84900d41a49a3ec
Author: Ian Lance Taylor <iant@golang.org>
Date:   Wed Jun 14 16:17:31 2023 -0700

    [release-branch.go1.20] text/template: set variables correctly in range assignment
    
    I unintentionally flipped them in CL 446795.
    
    For #56490
    For #60801
    Fixes #60802
    
    Change-Id: I57586bec052e1b2cc61513870ce24dd6ce17e56b
    Reviewed-on: https://go-review.googlesource.com/c/go/+/503576
    TryBot-Result: Gopher Robot <gobot@golang.org>
    Run-TryBot: Ian Lance Taylor <iant@google.com>
    Reviewed-by: Bryan Mills <bcmills@google.com>

I suspect the branch does not have CL 473415 fix. Otherwise I don't have explanation for the flake 2 days ago .

Alex

Sounds like we just need to backport the test fix to the 1.20 branch? I can handle that.

Change https://go.dev/cl/506975 mentions this issue: [release-branch.go1.20] runtime: allow for 5 more threads in TestWindowsStackMemory*

@gopherbot Please open backport issues for Go 1.19 and Go 1.20.

This is a test-only fix that is very small and very safe.

Backport issue(s) opened: #61054 (for 1.19), #61055 (for 1.20).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

The backport issues now track the backport fixes.

Change https://go.dev/cl/506976 mentions this issue: [release-branch.go1.19] runtime: allow for 5 more threads in TestWindowsStackMemory*

I have just hit this again with current tip: https://ci.chromium.org/ui/p/golang/builders/try/gotip-windows-386/b8766941830696722705/overview

Re-open the issue to see if it still be problem.

There's another one: https://ci.chromium.org/ui/p/golang/builders/try/gotip-windows-386/b8766892579985845249/overview

Sounds like we should increase the allowed threads from 5 to a bigger number, probably 10.

cc @prattmic @alexbrainman

There's another one: https://ci.chromium.org/ui/p/golang/builders/try/gotip-windows-386/b8766892579985845249/overview

Indeed.

Sounds like we should increase the allowed threads from 5 to a bigger number, probably 10.

@cuonglm why do you think 10 will work better than 5?

5 was selected as per this logic

Do you think runtime changed since CL 473415 (about 6 month ago)?

Thank you.

Alex

@cuonglm why do you think 10 will work better than 5?

5 was selected as per this logic

I saw the comment said that "sysmon and others", without explicitly said that what others are, so I assume we could increase if the test start flaking again.

Interestingly it always fails on windows-386, and never on windows-amd64.

@qmuntal is it possible that there are some extra threads that are started for every single process depending on a particular system used?

For example, every Go process loads a bunch of DLLs. Is it possible that some of those DLLs start their own threads? This could even vary from system to system.

Thank you.

Alex

I saw the comment said that "sysmon and others", without explicitly said that what others are,

I am pretty sure that sysmon is the only extra thread that is currently started - others will correct me. I added and others to future-proof my solution.

so I assume we could increase if the test start flaking again.

Yes. We could increase to 10 from 5.

Alex

Change https://go.dev/cl/536058 mentions this issue: runtime: allow for 10 more threads in TestWindowsStackMemory*

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "runtime" && test == "TestWindowsStackMemory"

=== RUN   TestWindowsStackMemory
    syscall_windows_test.go:671: C:\b\s\w\ir\x\t\go-build4036579385\testprog.exe StackMemory (28.3075ms): ok
    syscall_windows_test.go:677: expected < 102400 bytes of memory per thread, got 102884
--- FAIL: TestWindowsStackMemory (0.03s)

— watchflakes

Looking at this issue again, since we have another failure, our current guess is that getPagefileUsage probably includes memory other than stack memory, and it's occasionally pushing us over the edge. What exactly is being counted is unclear to us at this point, but this test seems like it's going to be inherently a bit flaky. We should dig into what exactly is being counted here.

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "runtime" && test == "TestWindowsStackMemory"

=== RUN   TestWindowsStackMemory
    syscall_windows_test.go:671: C:\b\s\w\ir\x\t\go-build580327174\testprog.exe StackMemory (24.111ms): ok
    syscall_windows_test.go:677: expected < 102400 bytes of memory per thread, got 102809
--- FAIL: TestWindowsStackMemory (0.02s)

— watchflakes