inconshreveable / muxado Goto Github PK

zeroConfig is a global variable, used when a nil config is supplied to Server or Client. initDefaults is then called on the config, modifying its fields. Thus there is a race condition if two goroutines simultaneously create a server or client with a nil config.

There are a few solutions:

1. Instantiate a new config on each call to Server or Client, instead of referencing a global variable
2. Declare a defaultConfig variable that holds the default values, and use that instead of zeroConfig when nil is passed
3. Use a mutex to serialize calls to initDefaults

Option 3 is clearly insane. Option 1 would be my choice, but I assume from zeroConfig that there is some motivation for reusing a global variable (efficiency?). If there is, option 2 seems like the best choice. I can open a PR for either of these.

The net package returns net.Error values in many cases. The net.Error interface includes a Timeout method, which allows the consumer to check explicitly for errors caused by a timeout. To better satisfy the goal of muxado to "feel just like the standard library's net package," its errors should better match the net package's errors.

When doing session.Accept() I'm getting an unexpected "Session closed".
Doing Wait() to get more info, I see this:

Error Code: 2
Error: "Illegal frame type: 0"
Debug: []

This seems to only happen when the last stream was being used for TLS, and it occurs immediately after that stream closes.

This code in proto/session.go:

func (a *netListenerAdaptor) Accept() (net.Conn, error) {
str, err := a.Accept()
return net.Conn(str), err
}

should be:

func (a *netListenerAdaptor) Accept() (net.Conn, error) {
str, err := a.Session.Accept()
return net.Conn(str), err
}

or you end up calling yourself over and over.

This is an awesome library with multiple ways to simplify our lives. If you can give complete working examples for both clients and servers (with heartbeat), it'll make implementation so much easier.

It's very hard to put together code from the readme.. Same with your other tunnel lib.

Please!

Hey @inconshreveable may want to merge muxado2/wip? Also, may be nice to have a spec for interoperable impls in other languages? even if just an example based listing of frames

And a more general thought: ideally, it could be http2-compliant so other languages could use http2 based muxers to talk to muxado, but not sure how much muxado departs from the spdy framing. maybe that ship's sailed and docked elsewhere by now.

Create a spec describing the protocol

Running the following code results in three possible outcomes:

• All streams are handled properly; program returns
• Deadlock
• Data race

package main

import (
    "net"
    "time"

    "github.com/inconshreveable/muxado"
)

func accept(sess muxado.Session) {
    for {
        stream, err := sess.Accept()
        if err != nil {
            println(err.Error())
            return
        }
        stream.Close()
    }
}

func open(sess muxado.Session) {
    stream, err := sess.Open()
    if err != nil {
        println(err.Error())
        return
    }
    stream.Close()
}

func main() {
    srvConn, cliConn := net.Pipe()
    srvSess := muxado.Server(srvConn)
    cliSess := muxado.Client(cliConn)

    go accept(srvSess)

    for i := 0; i < 10; i++ {
        go open(cliSess)
    }

    time.Sleep(time.Millisecond)

    srvSess.Close()
}

Stack trace:

==================
WARNING: DATA RACE
Write by main goroutine:
  runtime.closechan()
      /usr/local/go/src/runtime/chan.go:292 +0x0
  github.com/inconshreveable/muxado/proto.(*Session).die()
      /home/luke/go/src/github.com/inconshreveable/muxado/proto/session.go:243 +0x1a3
  github.com/inconshreveable/muxado/proto.(*Session).Close()
      /home/luke/go/src/github.com/inconshreveable/muxado/proto/session.go:171 +0x8e
  github.com/inconshreveable/muxado.(*sessionAdaptor).Close()
      <autogenerated>:42 +0x7a
  main.main()
      /home/luke/race.go:43 +0x6d1

Previous read by goroutine 9:
  runtime.chansend()
      /usr/local/go/src/runtime/chan.go:115 +0x0
  github.com/inconshreveable/muxado/proto.(*Session).handleFrame()
      /home/luke/go/src/github.com/inconshreveable/muxado/proto/session.go:343 +0x1dad
  github.com/inconshreveable/muxado/proto.(*Session).reader()
      /home/luke/go/src/github.com/inconshreveable/muxado/proto/session.go:288 +0x19b

Goroutine 9 (running) created at:
  github.com/inconshreveable/muxado/proto.NewSession()
      /home/luke/go/src/github.com/inconshreveable/muxado/proto/session.go:104 +0x876
  github.com/inconshreveable/muxado.Server()
      /home/luke/go/src/github.com/inconshreveable/muxado/server.go:42 +0x231
  main.main()
      /home/luke/race.go:32 +0x5d4
==================

Is this improper use of the library? Is it illegal to call Accept and Close concurrently? If so, how could I go about cleanly shutting down the session, given that Accept may block indefinitely?
If I close the underlying conn directly, the deadlocking and data race disappear, but for obvious reasons I'd prefer to avoid this.

In search for a TCP multiplexer I've found yamux which has many similarities with muxado so I'm wondering how they compare.

http://godoc.org/github.com/hashicorp/yamux

https://github.com/inconshreveable/tlsutil is not a public repo and testing WIP means skips some tests. Anyway to amke that avaiable?

I posted these at #ipfs on irc, but this may help others too:

1. can muxado handle other transports (seems it can just wrap a net.Conn, so yes?)
2. this example https://github.com/inconshreveable/muxado#a-more-extensive-muxado-client shows making one stream per-request. is this TRTTD, or -- if we have our own per-protocol framing -- do one stream per protocol (we have roughly 4 protocols each with their own messages in ipfs atm)
3. plans to make other language implementations?

The code

sess := muxado.Client(conn)

doesn't work. This function now has two arguments.

I'm closing this. It seems there was an issue with my code. muxado is great!

I need to use this protocol with a 100ms and 10MB/s link.
So I set the window size to 10MB*0.1*2 = 2MB.
In this code https://github.com/inconshreveable/muxado/blob/master/proto/buffer/circular.go#L38
muxado need 4MB memory for each connect.
I think this buffer should expend as needed, and has a max number which is window size.

package main

import (
	"log"
	"net"
	"sync"

	"github.com/inconshreveable/muxado"
)

func main() {
	a, b := net.Pipe()
	cli := muxado.Client(a, nil)
	ser := muxado.Server(b, nil)
	var wg sync.WaitGroup
	defer wg.Wait()
	wg.Add(1)
	go func() {
		defer wg.Done()
		x, err := cli.Open()
		if err != nil {
			log.Fatalf("cli.Open: %s.", err)
		}
		buf := make([]byte, 5)
		_, err = x.Read(buf)
		log.Printf("x.Read: %s.", err)
	}()
	wg.Add(1)
	go func() {
		defer wg.Done()
		y, err := ser.Accept()
		if err != nil {
			log.Fatalf("ser.Accept: %s.", err)
		}
		y.Close()
	}()
}

If a client sends something over the connection, it works as expected (client gets io.EOF).
If the connection is closed by the client rather than the server (and the server reads), it also works.

I did a go get github.com/inconshreveable/muxado and got the following errors:

 /usr/local/go/src/pkg/github.com/inconshreveable/muxado/proto/session.go:93: method sess.isClient is not an expression, must be called
 /usr/local/go/src/pkg/github.com/inconshreveable/muxado/proto/session.go:96: method sess.isServer is not an expression, must be called

Same errors if I import into a Go app and run

A stream muxer stress test i have fails when using muxado, it appears to hang after opening and closing a certain number of streams.

To reproduce:

go get -u github.com/whyrusleeping/go-smux-muxado
cd $GOPATH/src/github.com/whyrusleeping/go-smux-muxado && go test

To make the problem a little more visible, add this print line to jbenet/go-stream-muxer:

diff --git a/test/ttest.go b/test/ttest.go
index d7523b5..29ebb79 100644
--- a/test/ttest.go
+++ b/test/ttest.go
@@ -348,6 +348,7 @@ func SubtestStreamOpenStress(t *testing.T, tr smux.Transport) {
        for i := 0; i < count*5; i++ {
                select {
                case <-recv:
+                       fmt.Println("got stream: ", i)
                case <-limit:
                        t.Fatal("timed out receiving streams")
                }

I did a go get github.com/inconshreveable/muxado and got the following errors:

 /usr/local/go/src/pkg/github.com/inconshreveable/muxado/proto/session.go:93: method sess.isClient is not an expression, must be called
 /usr/local/go/src/pkg/github.com/inconshreveable/muxado/proto/session.go:96: method sess.isServer is not an expression, must be called

Same errors if I import into a Go app and run

Sorry about the repost, I wanted it to go from my personal a/c.

Currently having an issue where I have thousands of heartbeat go routines running since they do not seem to exit properly. Here is an example stack trace:

goroutine 1802 [chan receive]:
github.com/inconshreveable/muxado/proto.func·003(0xc210349460, 0x7f7b11975f38, 0x6ce720, 0xc21034d230)
        /opt/gopath/src/github.com/inconshreveable/muxado/proto/session.go:423 +0x32
github.com/inconshreveable/muxado/proto/ext.(*Heartbeat).respond(0xc2105030c0)
        /opt/gopath/src/github.com/inconshreveable/muxado/proto/ext/heartbeat.go:73 +0x54
created by github.com/inconshreveable/muxado/proto/ext.(*Heartbeat).Start
        /opt/gopath/src/github.com/inconshreveable/muxado/proto/ext/heartbeat.go:46 +0x48

Looks like the issue is that the extension acceptor routine does not account for the session being closed.