c/ooni-probe-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

ooni.go: use 32 bit counter to signal interruption (#120)

Browse Source 
Using a 64 bit counter has pitfalls. See Go documentation.

I don't want a refactoring or whatever to let these pitfalls
emerge in the future.

We just need one bit to signal we're done.

So use 32 bit, which shall be safe everywhere.

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

Proactively triggered by https://github.com/ooni/probe-engine/issues/399.
This commit is contained in:
Simone Basso 2020-03-18 12:32:53 +01:00 committed by GitHub
parent fe0b4a02be
commit 1d3e7e11ae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 7 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
ooni.go
View File

@ -31,9 +31,11 @@ type Context struct {
dbPath string dbPath string
configPath string configPath string
// We need to use a int64 in order to use the atomic.AddInt64/LoadInt64 // We need to use a int32 in order to use the atomic.AddInt32/LoadInt32
// operations to ensure consistent reads of the variables. // operations to ensure consistent reads of the variables. We do not use
isTerminatedAtomicInt int64 // a 64 bit integer here because that may lead to crashes with 32 bit
// OSes as documented in https://golang.org/pkg/sync/atomic/#pkg-note-BUG.
isTerminatedAtomicInt int32
} }
// MaybeLocationLookup will lookup the location of the user unless it's already cached // MaybeLocationLookup will lookup the location of the user unless it's already cached
@ -44,13 +46,13 @@ func (c *Context) MaybeLocationLookup() error {
// IsTerminated checks to see if the isTerminatedAtomicInt is set to a non zero // IsTerminated checks to see if the isTerminatedAtomicInt is set to a non zero
// value and therefore we have received the signal to shutdown the running test // value and therefore we have received the signal to shutdown the running test
func (c *Context) IsTerminated() bool { func (c *Context) IsTerminated() bool {
i := atomic.LoadInt64(&c.isTerminatedAtomicInt) i := atomic.LoadInt32(&c.isTerminatedAtomicInt)
return i != 0 return i != 0
} }
// Terminate interrupts the running context // Terminate interrupts the running context
func (c *Context) Terminate() { func (c *Context) Terminate() {
atomic.AddInt64(&c.isTerminatedAtomicInt, 1) atomic.AddInt32(&c.isTerminatedAtomicInt, 1)
} }
// ListenForSignals will listen for SIGINT and SIGTERM. When it receives those // ListenForSignals will listen for SIGINT and SIGTERM. When it receives those