Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Fix atomic usage on 32-bit systems #260

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
Loading
from
Open

Fix atomic usage on 32-bit systems #260

wants to merge 1 commit into from

Conversation

guillemj
Copy link

The atomic package requires aligned variables (as per the package documentation), but this is not happening for the variables on the stack. Instead change the types for these to 32-bit variables as there should be no need to track 64-bit stats.

This fixes various panics from the unaligned memory accesses, such as the one seen at https://ci.debian.net/data/autopkgtest/testing/i386/g/golang-github-allegro-bigcache/9212366/log.gz.

@guillemj
Fix atomic usage on 32-bit systems
e1c66c5 
The atomic package requires aligned variables (as per the package
documentation), but this is not happening for the variables on the
stack. Instead change the types for these to 32-bit variables as
there should be no need to track 64-bit stats.

This fixes various panics from the unaligned memory accesses.
@janisz
Copy link
Collaborator

janisz commented Jan 13, 2021

Just coping the linked panic log just to keep it here

=== RUN   TestRequestMetrics
--- FAIL: TestRequestMetrics (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x804a64c]

goroutine 6 [running]:
testing.tRunner.func1.1(0x81fc4a0, 0x83711b0)
	/usr/lib/go-1.15/src/testing/testing.go:1072 +0x291
testing.tRunner.func1(0x9400ee0)
	/usr/lib/go-1.15/src/testing/testing.go:1075 +0x358
panic(0x81fc4a0, 0x83711b0)
	/usr/lib/go-1.15/src/runtime/panic.go:969 +0x15c
runtime/internal/atomic.Xadd64(0x15afde1c, 0x1, 0x0, 0x904fefb3, 0x838c580)
	/usr/lib/go-1.15/src/runtime/internal/atomic/asm_386.s:105 +0xc
github.com/allegro/bigcache.(*cacheShard).miss(...)
	/tmp/autopkgtest-lxc.71tbek_l/downtmp/autopkgtest_tmp/_build/src/github.com/allegro/bigcache/shard.go:398
github.com/allegro/bigcache.(*cacheShard).getWrappedEntry(0x15afdd90, 0xd01cb2ae, 0x904fefb3, 0x1c, 0x50, 0x0, 0x0, 0x0)
	/tmp/autopkgtest-lxc.71tbek_l/downtmp/autopkgtest_tmp/_build/src/github.com/allegro/bigcache/shard.go:88 +0x10a
github.com/allegro/bigcache.(*cacheShard).get(0x15afdd90, 0x822723b, 0x5, 0xd01cb2ae, 0x904fefb3, 0x80a8a97, 0x2006e4cc, 0x899d1, 0x3ae7c6cc, 0x3ae7c6cc)
	/tmp/autopkgtest-lxc.71tbek_l/downtmp/autopkgtest_tmp/_build/src/github.com/allegro/bigcache/shard.go:64 +0x4d
github.com/allegro/bigcache.(*BigCache).Get(0x94b6180, 0x822723b, 0x5, 0x0, 0x3ae7c6cc, 0x2006e4cc, 0x899d1, 0x0)
	/tmp/autopkgtest-lxc.71tbek_l/downtmp/autopkgtest_tmp/_build/src/github.com/allegro/bigcache/bigcache.go:117 +0x76
github.com/allegro/bigcache/server.getCacheHandler(0x8265b20, 0x1c3b6f00, 0x94b6280)
	/tmp/autopkgtest-lxc.71tbek_l/downtmp/autopkgtest_tmp/_build/src/github.com/allegro/bigcache/server/cache_handlers.go:32 +0x5d
github.com/allegro/bigcache/server.cacheIndexHandler.func1(0x8265b20, 0x1c3b6f00, 0x94b6280)
	/tmp/autopkgtest-lxc.71tbek_l/downtmp/autopkgtest_tmp/_build/src/github.com/allegro/bigcache/server/cache_handlers.go:14 +0x77
net/http.HandlerFunc.ServeHTTP(0x8237164, 0x8265b20, 0x1c3b6f00, 0x94b6280)
	/usr/lib/go-1.15/src/net/http/server.go:2042 +0x34
github.com/allegro/bigcache/server.requestMetrics.func1.1(0x8265b20, 0x1c3b6f00, 0x94b6280)
	/tmp/autopkgtest-lxc.71tbek_l/downtmp/autopkgtest_tmp/_build/src/github.com/allegro/bigcache/server/middleware.go:25 +0x76
net/http.HandlerFunc.ServeHTTP(0x1d0780e0, 0x8265b20, 0x1c3b6f00, 0x94b6280)
	/usr/lib/go-1.15/src/net/http/server.go:2042 +0x34
github.com/allegro/bigcache/server.TestRequestMetrics(0x9400ee0)
	/tmp/autopkgtest-lxc.71tbek_l/downtmp/autopkgtest_tmp/_build/src/github.com/allegro/bigcache/server/middleware_test.go:41 +0x1fe
testing.tRunner(0x9400ee0, 0x823715c)
	/usr/lib/go-1.15/src/testing/testing.go:1123 +0xb7
created by testing.(*T).Run
	/usr/lib/go-1.15/src/testing/testing.go:1168 +0x211
FAIL	github.com/allegro/bigcache/server	0.120s
FAIL
dh_auto_test: error: cd _build && go test -vet=off -v -p 2 github.com/allegro/bigcache github.com/allegro/bigcache/queue github.com/allegro/bigcache/server returned exit code 1
make: *** [debian/rules:4: build] Error 25
autopkgtest [21:39:13]: test dh-golang-autopkgtest: -----------------------]
autopkgtest [21:39:13]: test dh-golang-autopkgtest:  - - - - - - - - - - results - - - - - - - - - -
dh-golang-autopkgtest FAIL non-zero exit status 2
autopkgtest [21:39:13]: @@@@@@@@@@@@@@@@@@@@ summary
dh-golang-autopkgtest FAIL non-zero exit status 2

@janisz
Copy link
Collaborator

janisz commented Jan 13, 2021

"package requires aligned variables (as per the package documentation)"
@guillemj Can you link to doc?

I'm not sure if going 32 bit is good solution. I understand this may solve the problem but it's limiting the statistics maybe we should consider mutex and locking? We can use channel to and additional gorutine to make this asyc to not slow down Read/Write operations.

@guillemj
Copy link
Author

"package requires aligned variables (as per the package documentation)"
@guillemj Can you link to doc?

https://golang.org/pkg/sync/atomic/#pkg-note-BUG

I'm not sure if going 32 bit is good solution. I understand this may solve the problem but it's limiting the statistics maybe we should consider mutex and locking? We can use channel to and additional gorutine to make this asyc to not slow down Read/Write operations.

I pondered about the reduced tracking, but then questioned whether the code would be reaching 2147483648 hits/misses?

Bornholm pushed a commit to Bornholm/bigcache that referenced this pull request Dec 1, 2023
feat: use atomic.Int64 in Stats to prevent 64bits alignment problems
2468f9b 
ref allegro#260
ref allegro#368
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@guillemj @janisz
Morty Proxy This is a proxified and sanitized view of the page, visit original site.