Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Prevent crash when loading zoneinfo on Windows #1112

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

alanhamlett
Copy link
Member

Failed to load advapi32.dll: Invalid access to memory location.

Sometimes loading zoneinfo on Windows causes a panic, not sure why but it's best to log the error and continue sending heartbeats instead of crashing.

Error Stacktrace
goroutine 1 [running]:
runtime/debug.Stack()
C:/hostedtoolcache/windows/go/1.22.5/x64/src/runtime/debug/stack.go:24 +0x5e
github.com/wakatime/wakatime-cli/cmd.runCmd.func1()
D:/a/wakatime-cli/wakatime-cli/cmd/run.go:297 +0x148
panic({0x1321320?, 0xc000041470?})
C:/hostedtoolcache/windows/go/1.22.5/x64/src/runtime/panic.go:770 +0x132
syscall.(*LazyProc).mustFind(...)
C:/hostedtoolcache/windows/go/1.22.5/x64/src/syscall/dll_windows.go:269
syscall.(*LazyProc).Addr(...)
C:/hostedtoolcache/windows/go/1.22.5/x64/src/syscall/dll_windows.go:276
syscall.RegOpenKeyEx(0x80000002, 0x37?, 0x0, 0x9, 0xc00006e000?)
C:/hostedtoolcache/windows/go/1.22.5/x64/src/syscall/zsyscall_windows.go:317 +0xae
internal/syscall/windows/registry.OpenKey(0x80000002, {0x13f8e95?, 0x1b1a900?}, 0x9)
C:/hostedtoolcache/windows/go/1.22.5/x64/src/internal/syscall/windows/registry/key.go:84 +0x66
time.toEnglishName({0xc00017b1d0, 0x12}, {0xc0001db400, 0xf})
C:/hostedtoolcache/windows/go/1.22.5/x64/src/time/zoneinfo_windows.go:59 +0x73
time.abbrev(0xc0004ac374)
C:/hostedtoolcache/windows/go/1.22.5/x64/src/time/zoneinfo_windows.go:96 +0xab
time.initLocalFromTZI(0xc0004ac374)
C:/hostedtoolcache/windows/go/1.22.5/x64/src/time/zoneinfo_windows.go:148 +0xbd
time.initLocal()
C:/hostedtoolcache/windows/go/1.22.5/x64/src/time/zoneinfo_windows.go:236 +0x8f
sync.(*Once).doSlow(0x12df6c0?, 0xc0004ac530?)
C:/hostedtoolcache/windows/go/1.22.5/x64/src/sync/once.go:74 +0xc2
sync.(*Once).Do(...)
C:/hostedtoolcache/windows/go/1.22.5/x64/src/sync/once.go:65
time.(*Location).get(0x1b19720)
C:/hostedtoolcache/windows/go/1.22.5/x64/src/time/zoneinfo.go:96 +0x45
time.(*Location).lookup(0x0?, 0x670688a8)
C:/hostedtoolcache/windows/go/1.22.5/x64/src/time/zoneinfo.go:150 +0x1c
time.parseRFC3339[...]({0xc0000fce21?, 0xc0000fce21?}, 0x1b19720?)
C:/hostedtoolcache/windows/go/1.22.5/x64/src/time/format_rfc3339.go:146 +0x556
time.Parse({0x13de194, 0x19}, {0xc0000fce21, 0x19})
C:/hostedtoolcache/windows/go/1.22.5/x64/src/time/format.go:1010 +0xbb
github.com/wakatime/wakatime-cli/cmd/params.LoadOfflineParams(0xc00006f880)
D:/a/wakatime-cli/wakatime-cli/cmd/params/params.go:669 +0x2b6
github.com/wakatime/wakatime-cli/cmd/heartbeat.LoadParams(_)
D:/a/wakatime-cli/wakatime-cli/cmd/heartbeat/heartbeat.go:184 +0x250
github.com/wakatime/wakatime-cli/cmd/heartbeat.SendHeartbeats(0xc00006f880, {0xc00002eff0, 0x2f})
D:/a/wakatime-cli/wakatime-cli/cmd/heartbeat/heartbeat.go:73 +0x59
github.com/wakatime/wakatime-cli/cmd/heartbeat.Run(0xc00006f880)

@alanhamlett alanhamlett force-pushed the bugfix/panic-from-time-parse-syscall branch from f98fa50 to 33ae1eb Compare October 29, 2024 09:33
Copy link
Member

@gandarez gandarez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also these unit tests in params_internal_test.go

func TestSafeTimeParse(t *testing.T) {
	parsed, err := safeTimeParse(ini.DateFormat, "2024-01-13T13:35:58Z")
	require.NoError(t, err)

	assert.Equal(t, time.Date(2024, 1, 13, 13, 35, 58, 0, time.UTC), parsed)
}

func TestSafeTimeParse_Err(t *testing.T) {
	tests := map[string]struct {
		Input    string
		Expected string
	}{
		"empty string": {
			Input:    "",
			Expected: `parsing time "" as "2006-01-02T15:04:05Z07:00": cannot parse "" as "2006"`,
		},
		"invalid time": {
			Input:    "invalid",
			Expected: `parsing time "invalid" as "2006-01-02T15:04:05Z07:00": cannot parse "invalid" as "2006"`,
		},
	}

	for name, test := range tests {
		t.Run(name, func(t *testing.T) {
			parsed, err := safeTimeParse(ini.DateFormat, test.Input)
			require.Equal(t, time.Time{}, parsed)

			assert.EqualError(t, err, test.Expected)
		})
	}
}

cmd/params/params.go Outdated Show resolved Hide resolved
@alanhamlett alanhamlett force-pushed the bugfix/panic-from-time-parse-syscall branch from d40334f to 21a54a3 Compare October 29, 2024 18:06
@alanhamlett alanhamlett force-pushed the bugfix/panic-from-time-parse-syscall branch from 21a54a3 to 3592fcb Compare October 29, 2024 18:06
@alanhamlett alanhamlett requested a review from gandarez October 29, 2024 18:07
@alanhamlett alanhamlett merged commit 8153a4b into develop Oct 29, 2024
19 checks passed
@alanhamlett alanhamlett deleted the bugfix/panic-from-time-parse-syscall branch October 29, 2024 18:10
Copy link

codecov bot commented Oct 29, 2024

Codecov Report

Attention: Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 63.10%. Comparing base (2044e98) to head (3592fcb).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
cmd/params/params.go 62.50% 2 Missing and 1 partial ⚠️
@@             Coverage Diff             @@
##           develop    #1112      +/-   ##
===========================================
- Coverage    63.11%   63.10%   -0.01%     
===========================================
  Files          383      383              
  Lines        16564    16570       +6     
===========================================
+ Hits         10454    10457       +3     
- Misses        5541     5543       +2     
- Partials       569      570       +1     
Flag Coverage Δ
unittests 63.10% <62.50%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cmd/params/params.go 85.80% <62.50%> (-0.27%) ⬇️

@alanhamlett alanhamlett mentioned this pull request Oct 29, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants