Skip to content

make logging thread safe#64

Open
packethog wants to merge 1 commit intojwhited:mainfrom
packethog:fix/loggin
Open

make logging thread safe#64
packethog wants to merge 1 commit intojwhited:mainfrom
packethog:fix/loggin

Conversation

@packethog
Copy link
Copy Markdown

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/corebgp@v0.8.5/logger.go:20 +0xc0
  github.com/jwhited/corebgp.logf()
      /go/pkg/mod/github.com/jwhited/corebgp@v0.8.5/logger.go:26 +0x34
  github.com/jwhited/corebgp.(*peer).logTransition()
      /go/pkg/mod/github.com/jwhited/corebgp@v0.8.5/peer.go:92 +0x134
  github.com/jwhited/corebgp.(*peer).sendTransitionToFSM()
      /go/pkg/mod/github.com/jwhited/corebgp@v0.8.5/peer.go:111 +0xec
  github.com/jwhited/corebgp.(*peer).handleStateTransition()
      /go/pkg/mod/github.com/jwhited/corebgp@v0.8.5/peer.go:196 +0x338
  github.com/jwhited/corebgp.(*peer).run()
      /go/pkg/mod/github.com/jwhited/corebgp@v0.8.5/peer.go:269 +0x2f8
  github.com/jwhited/corebgp.(*peer).start.gowrap1()
      /go/pkg/mod/github.com/jwhited/corebgp@v0.8.5/peer.go:291 +0x34

Previous write at 0x0000011bb370 by goroutine 112:
  github.com/jwhited/corebgp.SetLogger()
      /go/pkg/mod/github.com/jwhited/corebgp@v0.8.5/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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant