Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: each Read, ReadFrom variants doesn't take care of a shared buffer which is used by multiple goroutines #8858

gopherbot opened this issue Oct 3, 2014 · 5 comments


Copy link

@gopherbot gopherbot commented Oct 3, 2014

by rpzrpzrpz:

[root@rpzcentos rpzcache]# go version
go version go1.3.1 linux/amd64

glen,grxadd, gerr := gconn.ReadFromUDP(gbuf[0:])

go funcprocessbytes(grxadd,gbuf[0:glen])

There is no way to call a go routine after calling ReadFromUDP since those bytes are not
safe in the go routine.  

The go routine will pass along the proper values in grxadd, but the bytes in gbuf 
are sometimes duplicated on successive calls to the funcprocessbytes( ).

Changing the code and removing the "go" directive, the bytes are processed OK
because the bytes are left in the network stack and each call to funcprocessbytes() is
called serially instead of in parallel.

I will try to work on a sample code program for this.
Copy link

@gopherbot gopherbot commented Oct 3, 2014

Comment 1 by rpzrpzrpz:

Here is a sample Google Go Code Plus Python Code to generate the test.
You must set up a linux server with 5 IP addresses on the ethernet port. I am sure you
could probably just change the ports around and use a single IP address, but that is not
the test that I ran to produce the bugs.
You will observe the "Mangled Bytes" error message and that will indicate that the UDP
payload does not match the IP Address.
##############GO CODE##################
// testudp project main.go
package main
import (
var gmax int = 0
var gstop bool = false
func main() {
    fmt.Println("TestUdp Starting up:", time.Now().UTC().String())
    gsigchan := make(chan os.Signal, 3)
    signal.Notify(gsigchan, os.Interrupt, syscall.SIGTERM)
    signal.Notify(gsigchan, os.Interrupt, syscall.SIGUSR1)
    go func() {
        fmt.Println("TestUDP SIGTERM:", time.Now().UTC().String())
        gstop = true
    go udplisten("", 8500, &gstop)
    for {
        time.Sleep(1000 * time.Millisecond)
        if gstop == true {
        //fmt.Println("TestUdp Running:", time.Now().UTC().String())
    fmt.Println("TestUdp Exiting:", time.Now().UTC().String())
    time.Sleep(100 * time.Millisecond)
func udplisten(gip string, gport int, gstop *bool) bool {
    //gip - IP Address will listen to
    //gport - Port we will listen on
    //gstop - pointer to control variable to terminate thread
    var gbuf [1024]byte
    if gip == "" {
        fmt.Println("Invalid IP Address")
        return false
    if gport < 3000 {
        fmt.Println("Invalid Port")
        return false
    if gstop == nil {
        fmt.Println("Invalid Stop variable")
        return false
    gsrvaddr := net.UDPAddr{
        Port: gport,
        IP:   net.ParseIP(gip),
    fmt.Println("UDP Listen ENTER:", gsrvaddr)
    for *gstop == false {
        gconn, gerr := net.ListenUDP("udp4", &gsrvaddr)
        if gerr != nil {
            fmt.Println("Cluster UDP Listen Error:", gerr)
            time.Sleep(1000 * time.Millisecond)
        for *gstop == false {
            //#give us 1 second to read the bytes
            glen, grxadd, gerr := gconn.ReadFromUDP(gbuf[0:])
            if gerr != nil {
                //Remove timeout from this loop
                if strings.Contains(gerr.Error(), "i/o timeout") == true {
                fmt.Println("UDP Read Error:", gerr)
            if grxadd == nil {
                time.Sleep(200 * time.Millisecond)
                fmt.Println("UDP No Address returned on RX")
            go udpcallbackfunc(gconn, grxadd, glen, gbuf[0:glen])
    return true
func udpcallbackfunc(grxconn *net.UDPConn, grxadd *net.UDPAddr, grxlen int, grx []byte) {
    gstr := string(grx)
    if gstr == grxadd.IP.String() {
        //Our grx payload is good
    fmt.Println("Bytes Mangled:", gstr, grxadd.IP.String())
    if gmax > 200 {
        gstop = true
##############PYTHON CODE##################
import time
from socket import *
from threading import Thread
gstop = 0
def udpcli(srvaddr,cliaddr,payload):
    sock = socket(AF_INET,SOCK_DGRAM)
    print "UDP Thread going..."
    print cliaddr
    print srvaddr
    print payload
    while True:
        #Just send it a lot
        if gstop == 1:
    print "UDP Thread stopping"
    print payload
saddr = ("",8500)
caddr = ("",8500)
ta = Thread(target=udpcli,args=(saddr,caddr,""))
caddr = ("",8500)
tb = Thread(target=udpcli,args=(saddr,caddr,""))
caddr = ("",8500)
tc = Thread(target=udpcli,args=(saddr,caddr,""))
caddr = ("",8500)
td = Thread(target=udpcli,args=(saddr,caddr,""))
print "Started UDP client"
while 1:
    if gstop == 1:
        print "Running UDP client"
    except KeyboardInterrupt:
        print "Keyboard Break"
        gstop = 1
print "Broken Out Break"
print "Stopped..."
Copy link

@gopherbot gopherbot commented Oct 3, 2014

Comment 2 by rpzrpzrpz:

If you want to "fix" the sample, just remove the word "go" in front of the callbackfunc
in the go program.
go udpcallbackfunc(gconn, grxadd, glen, gbuf[0:glen])
to --> udpcallbackfunc(gconn, grxadd, glen, gbuf[0:glen])
without the word "go" infront of it.
Copy link

@gopherbot gopherbot commented Oct 3, 2014

Comment 3 by rpzrpzrpz:

I did not mean to suggest that "fix" meant the bug was gone. Its just that a go routine
enabled server is out of the question.  
It would just be easier for the rx bytes to get copied correctly into the gbuf from
ReadFromUDP( ), but that does not seem to be the case.
Copy link

@minux minux commented Oct 3, 2014

Comment 4:

each udpcallback goroutine is using the same underlying backing array that is backing
gbuf, so data corruption is expected if you introduce concurrency.
run you code under the race detector and this bug will be pointed out.
go build -race

Status changed to WorkingAsIntended.

Copy link

@mikioh mikioh commented Oct 7, 2014

Comment 5:

Status changed to Duplicate.

Merged into issue #8881.

@gopherbot gopherbot added the duplicate label Oct 7, 2014
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.