Merged master

Signed-off-by: French Ben <frenchben@docker.com>
flynn · May 26, 2016 · 50a8a16 · 50a8a16
2 parents db96364 + 20882a7
commit 50a8a16
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 16 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -0,0 +1,11 @@
+language: go
+
+go:
+  - 1.6
+
+script:
+  - go test -coverprofile=coverage.txt -covermode=atomic -race
+
+# CODECOV_TOKEN set in travisCI ENV
+after_success:
+  - bash <(curl -s https://codecov.io/bash)
diff --git a/README.md b/README.md
@@ -1,4 +1,5 @@
-# p9p [![GoDoc](https://godoc.org/github.com/docker/go-p9p?status.svg)](https://godoc.org/github.com/docker/go-p9p) [![CircleCI](https://circleci.com/gh/docker/go-p9p.svg?style=shield)](https://circleci.com/gh/docker/go-p9p) [![Go Report Card](https://goreportcard.com/badge/github.com/docker/go-p9p)](https://goreportcard.com/report/github.com/docker/go-p9p) [![Badge Badge](http://badge-server.badge-validate.80d2e13b.svc.dockerapp.io:8088/github.com/docker/go-p9p)](https://github.com/docker/go-p9p)
+# p9p [![GoDoc](https://godoc.org/github.com/docker/go-p9p?status.svg)](https://godoc.org/github.com/docker/go-p9p) [![Apache licensed](https://img.shields.io/badge/license-Apache-blue.svg)](https://raw.githubusercontent.com/docker/go-p9p/master/LICENSE) [![CircleCI](https://circleci.com/gh/docker/go-p9p.svg?style=shield)](https://circleci.com/gh/docker/go-p9p) [![TravisCI](https://travis-ci.org/docker/go-p9p.svg?branch=master)](https://travis-ci.org/docker/go-p9p) [![Go Report Card](https://goreportcard.com/badge/github.com/docker/go-p9p)](https://goreportcard.com/report/github.com/docker/go-p9p)
+[![Badge Badge](http://badge-server.badge-validate.80d2e13b.svc.dockerapp.io:8088/github.com/docker/go-p9p)](https://github.com/docker/go-p9p)
 
 
 A modern, performant 9P library for Go.

diff --git a/fcall.go b/fcall.go
@@ -111,11 +111,6 @@ type Fcall struct {
 }
 
 func newFcall(tag Tag, msg Message) *Fcall {
-	switch msg.Type() {
-	case Tversion, Rversion:
-		tag = NOTAG
-	}
-
 	return &Fcall{
 		Type:    msg.Type(),
 		Tag:     tag,

diff --git a/transport.go b/transport.go
@@ -1,6 +1,7 @@
 package p9p
 
 import (
+	"errors"
 	"fmt"
 	"log"
 	"net"
@@ -30,7 +31,7 @@ type transport struct {
 
 var _ roundTripper = &transport{}
 
-func newTransport(ctx context.Context, ch *channel) roundTripper {
+func newTransport(ctx context.Context, ch Channel) roundTripper {
 	t := &transport{
 		ctx:      ctx,
 		ch:       ch,
@@ -95,6 +96,30 @@ func (t *transport) send(ctx context.Context, msg Message) (Message, error) {
 	}
 }
 
+// allocateTag returns a valid tag given a tag pool map. It receives a hint as
+// to where to start the tag search. It returns an error if the allocation is
+// not possible. The provided map must not contain NOTAG as a key.
+func allocateTag(r *fcallRequest, m map[Tag]*fcallRequest, hint Tag) (Tag, error) {
+	// The tag pool is depleted if 65535 (0xFFFF) tags are taken.
+	if len(m) >= 0xFFFF {
+		return 0, errors.New("tag pool depleted")
+	}
+
+	// Look for the first tag that doesn't exist in the map and return it.
+	for i := 0; i < 0xFFFF; i++ {
+		hint++
+		if hint == NOTAG {
+			hint = 0
+		}
+
+		if _, exists := m[hint]; !exists {
+			return hint, nil
+		}
+	}
+
+	return 0, errors.New("allocateTag: unexpected error")
+}
+
 // handle takes messages off the wire and wakes up the waiting tag call.
 func (t *transport) handle() {
 	defer func() {
@@ -104,9 +129,9 @@ func (t *transport) handle() {
 	// the following variable block are protected components owned by this thread.
 	var (
 		responses = make(chan *Fcall)
-		tags      Tag
 		// outstanding provides a map of tags to outstanding requests.
 		outstanding = map[Tag]*fcallRequest{}
+		selected    Tag
 	)
 
 	// loop to read messages off of the connection
@@ -150,14 +175,16 @@ func (t *transport) handle() {
 	for {
 		select {
 		case req := <-t.requests:
-			// BUG(stevvooe): This is an awful tag allocation procedure.
-			// Replace this with something that let's us allocate tags and
-			// associate data with them, returning to them to a pool when
-			// complete. Such a system would provide a lot of information
-			// about outstanding requests.
-			tags++
-			fcall := newFcall(tags, req.message)
-			outstanding[fcall.Tag] = req
+			var err error
+
+			selected, err = allocateTag(req, outstanding, selected)
+			if err != nil {
+				req.err <- err
+				continue
+			}
+
+			outstanding[selected] = req
+			fcall := newFcall(selected, req.message)
 
 			// TODO(stevvooe): Consider the case of requests that never
 			// receive a response. We need to remove the fcall context from