search

John-Lin / tinynet

A lightweight instant virtual network for rapid prototyping SDN
Apache License 2.0
45 stars 4 forks source link

Race Condition when execute `AddSwitch` multiple times #10

Closed hwchiu closed 7 years ago

hwchiu commented 7 years ago 
import (
    tn "github.com/John-Lin/tinynet"
)

// Custom topology example
// Single switch with 8 hosts

func main() {
    // add a switch as a Switch
    tn.AddSwitch("br0")
    tn.AddSwitch("br1")
    tn.AddSwitch("br2")
    //  _, _ = tn.AddSwitch("br1")
}

Assume the go file is test.go Clean the topology via clean.py before each go run execute via sudo -E env PATH=$PATH go run -race test.go will get following error

INFO[0001] Adding a switch: br0
==================
WARNING: DATA RACE
Write at 0x00c4202d0270 by main goroutine:
  runtime.mapassign_fast64()
      /usr/local/go/src/runtime/hashmap_fast.go:510 +0x0
  github.com/John-Lin/tinynet/vendor/github.com/contiv/libovsdb.configureConnection()
      /home/ubuntu/go/src/github.com/John-Lin/tinynet/vendor/github.com/contiv/libovsdb/client.go:22 +0x7fe
  github.com/John-Lin/tinynet/vendor/github.com/contiv/libovsdb.ConnectUnix()
      /home/ubuntu/go/src/github.com/John-Lin/tinynet/vendor/github.com/contiv/libovsdb/client.go:68 +0xd4
  github.com/John-Lin/tinynet/vendor/github.com/John-Lin/ovsdb.NewOvsDriverWithUnix()
      /home/ubuntu/go/src/github.com/John-Lin/tinynet/vendor/github.com/John-Lin/ovsdb/ovsdb.go:111 +0x63
  github.com/John-Lin/tinynet.NewOVSSwitch()
      /home/ubuntu/go/src/github.com/John-Lin/tinynet/ovs_switch.go:41 +0xdb
  github.com/John-Lin/tinynet.AddSwitch()
      /home/ubuntu/go/src/github.com/John-Lin/tinynet/net.go:59 +0x81
  main.main()
      /home/ubuntu/go/src/github.com/John-Lin/tinynet/qqq/test.go:13 +0xd2

Previous read at 0x00c4202d0270 by goroutine 17:
  runtime.mapaccess2_fast64()
      /usr/local/go/src/runtime/hashmap_fast.go:159 +0x0
  github.com/John-Lin/tinynet/vendor/github.com/contiv/libovsdb.update()
      /home/ubuntu/go/src/github.com/John-Lin/tinynet/vendor/github.com/contiv/libovsdb/client.go:143 +0x1ab
  runtime.call64()
      /usr/local/go/src/runtime/asm_amd64.s:510 +0x3a
  reflect.Value.Call()
      /usr/local/go/src/reflect/value.go:302 +0xc0
  github.com/John-Lin/tinynet/vendor/github.com/cenkalti/rpc2.(*Client).handleRequest()
      /home/ubuntu/go/src/github.com/John-Lin/tinynet/vendor/github.com/cenkalti/rpc2/client.go:131 +0x243

Goroutine 17 (finished) created at:
  github.com/John-Lin/tinynet/vendor/github.com/cenkalti/rpc2.(*Client).readRequest()
      /home/ubuntu/go/src/github.com/John-Lin/tinynet/vendor/github.com/cenkalti/rpc2/client.go:181 +0x3c0
  github.com/John-Lin/tinynet/vendor/github.com/cenkalti/rpc2.(*Client).readLoop()
      /home/ubuntu/go/src/github.com/John-Lin/tinynet/vendor/github.com/cenkalti/rpc2/client.go:93 +0x260
  github.com/John-Lin/tinynet/vendor/github.com/cenkalti/rpc2.(*Client).Run()
      /home/ubuntu/go/src/github.com/John-Lin/tinynet/vendor/github.com/cenkalti/rpc2/client.go:62 +0x38
==================
INFO[0002] Adding a switch: br1
INFO[0003] Adding a switch: br2
Found 1 data race(s)
exit status 66
hwchiu commented 7 years ago

I found that John-Lin/ovsdb use contiv/libovsdb and there's a global map variable on contriv/libovsdb/client.go. They don't do any lock procedure to protect map and that cause this problem. Besides, I also found the original repo of libovsdb "socketplane/libovsdb" has fix that problem via lock. I think John-Lin/ovsdb should change its dependency from contiv/libovsdb to socketplane/libovsdb. I will testing its function and send a PR to you if it works.

hwchiu commented 7 years ago

Close it since https://github.com/John-Lin/ovsdb/pull/8 has updated to fix this issue.