m3ua.Conn is not concurrency-safe / has data-races

[linouxis9]

Hi!

Here are a few issues I found with m3ua.Conn:

• messages.Data.MarshalBinary will call SetLength on its inner params.Param (eg. RoutingContexts) https://github.com/wmnsk/go-m3ua/blob/3084b69e489f734d959558e1d06699fe967a187a/conn.go#L94-L100 But these params.Param are globals from the cfg, so SetLength will be called concurrently on these params.Param from concurrent Write to the m3ua.Conn. Solution (in order of perf impact):

  • Call SetLength once / only when the params.Param is modified
  • Deep copy the params.Param from cfg before handing it to messages.NewData
  • Call conn.Mu.Lock() in WriteToStreamWithConfig

• c.cfg.HeartbeatInfo.Data is read and written concurrently without a mutex or an atomic pointer. https://github.com/wmnsk/go-m3ua/blob/3084b69e489f734d959558e1d06699fe967a187a/asptm.go#L135

IMHO, seeing these two issues; Conn.cfg should never be modifed by Conn, and only the user of a m3ua.Conn should be able to change the cfg.

• c.state is read and written concurrently (eg. in conn.Write and some ASP state updates) without c.state reads not always being protected by a Mutex. Solution (in order of perf impact):
  • Use atomic.Pointer[State] to hold the conn's State, and Load/Store everywhere needed
  • Call conn.Mu.Lock() every time c.state is read

I hope it'll help!

Cheers, Valentin