corebgp icon indicating copy to clipboard operation
corebgp copied to clipboard

make logging thread safe

Open packethog opened this issue 8 months ago • 0 comments

In the event you have two corebgp servers running in the same process, a data race can be created due to the logger being set as a package var:

WARNING: DATA RACE
Read at 0x0000011bb370 by goroutine 75:
  github.com/jwhited/corebgp.log()
      /go/pkg/mod/github.com/jwhited/[email protected]/logger.go:20 +0xc0
  github.com/jwhited/corebgp.logf()
      /go/pkg/mod/github.com/jwhited/[email protected]/logger.go:26 +0x34
  github.com/jwhited/corebgp.(*peer).logTransition()
      /go/pkg/mod/github.com/jwhited/[email protected]/peer.go:92 +0x134
  github.com/jwhited/corebgp.(*peer).sendTransitionToFSM()
      /go/pkg/mod/github.com/jwhited/[email protected]/peer.go:111 +0xec
  github.com/jwhited/corebgp.(*peer).handleStateTransition()
      /go/pkg/mod/github.com/jwhited/[email protected]/peer.go:196 +0x338
  github.com/jwhited/corebgp.(*peer).run()
      /go/pkg/mod/github.com/jwhited/[email protected]/peer.go:269 +0x2f8
  github.com/jwhited/corebgp.(*peer).start.gowrap1()
      /go/pkg/mod/github.com/jwhited/[email protected]/peer.go:291 +0x34

Previous write at 0x0000011bb370 by goroutine 112:
  github.com/jwhited/corebgp.SetLogger()
      /go/pkg/mod/github.com/jwhited/[email protected]/logger.go:16 +0x3c
  github.com/malbeclabs/doublezero/client/doublezerod/internal/bgp.NewBgpServer()
      /workspaces/doublezero/client/doublezerod/internal/bgp/bgp.go:105 +0x30
  github.com/malbeclabs/doublezero/client/doublezerod/internal/runtime.Run()
      /workspaces/doublezero/client/doublezerod/internal/runtime/run.go:20 +0xe4
  github.com/malbeclabs/doublezero/client/doublezerod/internal/runtime_test.TestEndToEnd_IBRL.func2.10()
      /workspaces/doublezero/client/doublezerod/internal/runtime/run_test.go:391 +0x74

This PR creates a new WithLogger option for the corebgp server constructor that allows for a logger to be set on a instance basis. This is then passed down through the peer constructor as well. The SetLogger function has been kept to maintain backwards compatibility.

packethog avatar May 05 '25 21:05 packethog