Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Handling read/write udp connection in Go

Tags:

go

udp

I need to create UDP connection through which I could write and read packets simultaneously. (using different goroutines and with GOMAXPROCS(n) where n>1) First attempt was something like this:

func new_conn(port, chan_buf int) (conn *net.UDPConn, inbound chan Packet, err error) {
    inbound = make(chan Packet, chan_buf)

    conn, err = net.ListenUDP("udp4", &net.UDPAddr{Port: port})
    if err != nil {return}

    go func () {
        for {
            b := make([]byte, UDP_PACKET_SIZE)
            n, addr, err := conn.ReadFromUDP(b)
            if err != nil {
                log.Printf("Error: UDP read error: %v", err)
                continue
            }
            inbound <- Packet{addr, b[:n]}
        }
    }
}

So to read packet I used packet := <- inbound and to write conn.WriteTo(data_bytes, remote_addr). But race detector issues warnings on simultaneous read/write on connection. So I rewrite code to something like this:

func new_conn(port, chan_buf int) (inbound, outbound chan Packet, err error) {
    inbound = make(chan Packet, chan_buf)
    outbound = make(chan Packet, chan_buf)

    conn, err = net.ListenUDP("udp4", &net.UDPAddr{Port: port})
    if err != nil {return}

    go func () {
        for {
            select {
            case packet := <- outbound:
                _, err := conn.WriteToUDP(packet.data, packet.addr)
                if err != nil {
                    log.Printf("Error: UDP write error: %v", err)
                    continue
                }
            default:
                b := make([]byte, UDP_PACKET_SIZE)
                n, addr, err := conn.ReadFromUDP(b)
                if err != nil {
                    log.Printf("Error: UDP read error: %v", err)
                    continue
                }
                inbound <- Packet{addr, b[:n]}
            }
        }
    }
}

This code will no more trigger race condition, but have risk of blocking goroutine if there is no inbound packets. Only solution which I see is to call something like SetReadDeadline(time.Now()+10*time.Millisecond) before calling ReadFromUDP. This code will probably work, but I don't like it so much. Is there more elegant ways to solve this problem?

UPD: Warning message:

==================
WARNING: DATA RACE
Read by goroutine 553:
  net.ipToSockaddr()
      /usr/local/go/src/pkg/net/ipsock_posix.go:150 +0x18a
  net.(*UDPAddr).sockaddr()
      /usr/local/go/src/pkg/net/udpsock_posix.go:45 +0xd9
  net.(*UDPConn).WriteToUDP()
      /usr/local/go/src/pkg/net/udpsock_posix.go:123 +0x4df
  net.(*UDPConn).WriteTo()
      /usr/local/go/src/pkg/net/udpsock_posix.go:139 +0x2f6
  <traceback which points on conn.WriteTo call>

Previous write by goroutine 556:
  syscall.anyToSockaddr()
      /usr/local/go/src/pkg/syscall/syscall_linux.go:383 +0x336
  syscall.Recvfrom()
      /usr/local/go/src/pkg/syscall/syscall_unix.go:223 +0x15c
  net.(*netFD).ReadFrom()
      /usr/local/go/src/pkg/net/fd_unix.go:227 +0x33c
  net.(*UDPConn).ReadFromUDP()
      /usr/local/go/src/pkg/net/udpsock_posix.go:67 +0x164
  <traceback which points on conn.ReadFromUDP call>

Goroutine 553 (running) created at:
  <traceback>

Goroutine 556 (running) created at:
  <traceback>
==================
like image 527
user3234005 Avatar asked Feb 23 '14 12:02

user3234005


2 Answers

According to the trace from the race detector, the detected race appears to be due to the reuse of a UDPAddr returned by a read call in a subsequent write. In particular, the data its IP field references.

It's not clear that this is really a problem though, since syscall.ReadFrom is allocating a new address structure on every call and doesn't hold on to that structure long term. You could try copying the address prior to sending it to your outbound goroutine. For example:

newAddr := new(net.UDPAddr)
*newAddr = *addr
newAddr.IP = make(net.IP, len(addr.IP))
copy(newAddr.IP, add.IP)

But without knowing more about your program, it is difficult to tell why this is being flagged as a race. Perhaps it is enough to point you in the right direction though. I wasn't able to reproduce the race using this test program based on what you've posted: http://play.golang.org/p/suDG6hCYYP

like image 185
James Henstridge Avatar answered Sep 30 '22 04:09

James Henstridge


Why not start two goroutines, one for writing and one for reading and be full duplex? i.e:

func new_conn(port, chan_buf int) (inbound, outbound chan Packet, err error) {
    inbound = make(chan Packet, chan_buf)
    outbound = make(chan Packet, chan_buf)

    conn, err = net.ListenUDP("udp4", &net.UDPAddr{Port: port})
    if err != nil {
        return
    }

    go func() {
        for packet := range outbound {
            _, err := conn.WriteToUDP(packet.data, packet.addr)
            if err != nil {
                log.Printf("Error: UDP write error: %v", err)
                continue
            }
        }
    }()

    go func() {

        b := make([]byte, UDP_PACKET_SIZE)
        for {

            n, addr, err := conn.ReadFromUDP(b)
            if err != nil {
                log.Printf("Error: UDP read error: %v", err)
                continue
            }
            b2 := make([]byte, UDP_PACKET_SIZE)
            copy(b2, b)
            inbound <- Packet{addr, b2[:n]}
        }
    }()

}
like image 44
Not_a_Golfer Avatar answered Sep 30 '22 04:09

Not_a_Golfer