aidantwoods / go-paseto

Platform-Agnostic Security Tokens implementation in Golang.
https://pkg.go.dev/aidanwoods.dev/go-paseto
MIT License
322 stars 17 forks source link

Data race #54

Open DarkHeros09 opened 1 month ago

DarkHeros09 commented 1 month ago

Hi

thank you for making this package. I've been using it for a month or so now, and i noticed that i get data race error from time to time

Fatal error: concurrent map writes

WARNING: DATA RACE
Write at 0x00c0007fa1e8 by goroutine 335:
  aidanwoods.dev/go-paseto.(*Token).Set.func1()
      C:/Users/m_ben/go/pkg/mod/aidanwoods.dev/go-paseto@v1.5.2/token.go:67 +0x7b
  aidanwoods.dev/go-result.Match[go.shape.struct { aidanwoods.dev/go-result.value aidanwoods.dev/go-result.Option[go.shape.interface {}]; aidanwoods.dev/go-result.err aidanwoods.dev/go-result.Option[error] },go.shape.[]uint8]()
      C:/Users/m_ben/go/pkg/mod/aidanwoods.dev/go-result@v0.1.0/result.go:105 +0xfd
  aidanwoods.dev/go-result.AndThen[go.shape.[]uint8,go.shape.interface {}]()
      C:/Users/m_ben/go/pkg/mod/aidanwoods.dev/go-result@v0.1.0/result.go:120 +0xe4
  aidanwoods.dev/go-result.ApplyToResult[go.shape.interface {},go.shape.[]uint8].AndThen()
      C:/Users/m_ben/go/pkg/mod/aidanwoods.dev/go-result@v0.1.0/chaining.go:14 +0x132
  aidanwoods.dev/go-paseto.(*Token).Set()

i solved it by using sync.mutex, but i don't know if that's the right solution or whether will it affect my backend performance.

i don't know if implementing syncmap.Map{} will fix this issue.

mologie commented 1 month ago

(Hey! I'm not the maintainer, just a user who saw your issue:)

This library neither uses Goroutines nor does any synchronization for you. The interface is therefore not thread-safe, which is why your application correctly trips the race detector.

If you call .Set() on a token from multiple Goroutines, then you need to explicitly synchronize access to it, just like with any other datastructure in Go. This is not an issue in this library. Using sync.Mutex is the right way to do that. Another way would be to structure your application so that only a single Goroutine constructs the token.

DarkHeros09 commented 1 month ago

(Hey! I'm not the maintainer, just a user who saw your issue:)

This library neither uses Goroutines nor does any synchronization for you. The interface is therefore not thread-safe, which is why your application correctly trips the race detector.

If you call .Set() on a token from multiple Goroutines, then you need to explicitly synchronize access to it, just like with any other datastructure in Go. This is not an issue in this library. Using sync.Mutex is the right way to do that. Another way would be to structure your application so that only a single Goroutine constructs the token.

Hi Thank you for your reply. I already used sync.Mutex and i still get the same error.

I reverted back to o1egl/paseto package since it didn't have this problem.

I hope this issue get solved.

mologie commented 1 month ago

Please post a reproducer code snippet. We're a heavy user of this lib too so I'm interested in it having no critical bugs, would investigate it based on your code. The tests of the lib pass fine under the Go race checker in the environments I have here (Ubuntu 24.04 amd64, macOS 14.7 arm64).

aidantwoods commented 1 month ago

I'll second the request for a repro snippet, ideally showing how you're using sync.Mutex to set properties on a token.

This library (like Go's own data structures such as map[_]_) isn't designed to handle data races transparently. Instead if you're using multiple goroutines to write to a single token, it would be up to you to impose locks to ensure concurrent writes don't happen.

Outside of race safety with the underlying data structures, concurrent writes may also cause issues (e.g. if two processes write to the same key, the order of writes would change the end result). For this reason, I'm not sure it's possible for this library to make general promises about race safety when building a token (since there are logic problems that can occur, rather than just data structure related ones).

DarkHeros09 commented 1 month ago

My codebase is quite large, so I'll work on producing a minimal code snippet when I get the chance. It might take a little while, as I'm tied up with other tasks at the moment, but I'll get back to you when I can.