Unity-Technologies / go-ts3

A golang TeamSpeak 3 ServerQuery client
BSD 2-Clause "Simplified" License
46 stars 20 forks source link

Freezing in go routine #21

Open Blendman974 opened 5 years ago

Blendman974 commented 5 years ago

The program hangs when this api in repeatedly used in a go routine. See following code :

package main

import (
    "fmt"
    "github.com/multiplay/go-ts3"
)

func main() {

    conn, err := ts3.NewClient("ts.foo.bar:10011")

    if err != nil {
        fmt.Println("ERROR : ", err)
    }

    if err := conn.Login("foo", "bar"); err != nil {
        fmt.Println("ERROR : ", err)
    }

    if err := conn.Use(1); err != nil {
        fmt.Println("ERROR : ", err)
    }

    go loop(conn) //Here if you remove the "go", the program will work as expected. 

    for {

    }
}

func loop(conn *ts3.Client){
    for  {

        //time.Sleep(time.Second*10) //waiting doesn't matter, you may wait an hour it will still hang

        conn.Server.GroupList() //The program will hang after 4 iteration of GroupList() or 40-50 iteration of ClientDBList() but all commands eventually hang after a while
        fmt.Println("Tick")

    }
}
irgendwr commented 5 years ago

Check if it's sill reproducible with the current version, my PR changed how stuff get's handled. (go get -u ...)

RonMelkhior commented 5 years ago

I have managed to reproduce the issue as well (with the latest commit @irgendwr).
The issue happens when the connection times out/closes, but it isn't recognized by IsConnected().

For me, the issue comes from here:
https://github.com/multiplay/go-ts3/blob/master/client.go#L237

Pushing to c.work blocks it, which in itself is caused because of this function call:
https://github.com/multiplay/go-ts3/blob/master/client.go#L208

c.process freezes inside, when trying to call conn.Write().

Blendman974 commented 5 years ago

:+1: But why does it freezes only while executed in a go routine ?

irgendwr commented 5 years ago

Check if this changes anything: https://github.com/irgendwr/go-ts3/commit/2516d1055a7a24e17ba953a5e8791690a51c29c8 (you'll have to copy the changes)

RonMelkhior commented 5 years ago

I'll test it out today, it'll probably work. But I think a better way to tackle this is to fix conn.Write() freezing in the first place.

I fixed it on my end by doing:

if err == ts3Lib.ErrTimeout || err == ts3Lib.ErrNotConnected {
    conn.Timeout()
} else if netErr, ok := err.(net.Error); ok && netErr.Timeout() {
    conn.Timeout()
}

You can apply the net.Error check in the library codebase, and if it's true fix IsConnected()

irgendwr commented 5 years ago

I think a better way to tackle this is to fix conn.Write() freezing in the first place.

conn.Write should not freeze longer than the specified timeout because SetWriteDeadline is used.

It was likely freezing because the client was reading two responses (because no mutex was used) and then waiting for the response it already received until the deadline is reached.

DefaultTimeout is 10 * time.Second, if you want a shorter timeout set the option when creating a client:

client, err := NewClient(addr, Timeout(time.Millisecond*200))

You can apply the net.Error check in the library codebase

I'm not a maintainer btw, I have no permissions ^^