Repository: pion/turn (go)

Issue:
Too many open files bug
After running the server for a day and getting a regular influx of users, I was unable to use the TURN server and got many of these errors in the console: 

```
2018/12/07 17:32:57 Failed to handle ALLOCATE-REQUEST from x.x.x.x:37291: listen udp4 0.0.0.0:0: socket: too many open files
```

Pull request:
Handle refresh with value of 0
Delete allocation if this value is received

Resolves #27


Patch:
diff --git a/internal/server/turn.go b/internal/server/turn.go
index bd3a73a..5ea706a 100644
--- a/internal/server/turn.go
+++ b/internal/server/turn.go
@@ -182,17 +182,23 @@ func handleRefreshRequest(r Request, m *stun.Message) error {
 		return err
 	}
 
-	a := r.AllocationManager.GetAllocation(&allocation.FiveTuple{
+	lifetimeDuration := allocationLifeTime(m)
+	fiveTuple := &allocation.FiveTuple{
 		SrcAddr:  r.SrcAddr,
 		DstAddr:  r.Conn.LocalAddr(),
 		Protocol: allocation.UDP,
-	})
-	if a == nil {
-		return fmt.Errorf("no allocation found for %v:%v", r.SrcAddr, r.Conn.LocalAddr())
 	}
 
-	lifetimeDuration := allocationLifeTime(m)
-	a.Refresh(lifetimeDuration)
+	if lifetimeDuration != 0 {
+		a := r.AllocationManager.GetAllocation(fiveTuple)
+
+		if a == nil {
+			return fmt.Errorf("no allocation found for %v:%v", r.SrcAddr, r.Conn.LocalAddr())
+		}
+		a.Refresh(lifetimeDuration)
+	} else {
+		r.AllocationManager.DeleteAllocation(fiveTuple)
+	}
 
 	return buildAndSend(r.Conn, r.SrcAddr, buildMsg(m.TransactionID, stun.NewType(stun.MethodRefresh, stun.ClassSuccessResponse), []stun.Setter{
 		&proto.Lifetime{
diff --git a/server.go b/server.go
index 919d19e..d6be792 100644
--- a/server.go
+++ b/server.go
@@ -99,7 +99,6 @@ func (s *Server) Close() error {
 }
 
 func (s *Server) connReadLoop(c net.Conn, r RelayAddressGenerator) {
-	buf := make([]byte, inboundMTU)
 	allocationManager, err := allocation.NewManager(allocation.ManagerConfig{
 		AllocatePacketConn: r.AllocatePacketConn,
 		AllocateConn:       r.AllocateConn,
@@ -116,6 +115,7 @@ func (s *Server) connReadLoop(c net.Conn, r RelayAddressGenerator) {
 	}()
 
 	stunConn := NewSTUNConn(c)
+	buf := make([]byte, inboundMTU)
 	for {
 		n, addr, err := stunConn.ReadFrom(buf)
 
@@ -140,7 +140,6 @@ func (s *Server) connReadLoop(c net.Conn, r RelayAddressGenerator) {
 }
 
 func (s *Server) packetConnReadLoop(p net.PacketConn, r RelayAddressGenerator) {
-	buf := make([]byte, inboundMTU)
 	allocationManager, err := allocation.NewManager(allocation.ManagerConfig{
 		AllocatePacketConn: r.AllocatePacketConn,
 		AllocateConn:       r.AllocateConn,
@@ -156,6 +155,7 @@ func (s *Server) packetConnReadLoop(p net.PacketConn, r RelayAddressGenerator) {
 		}
 	}()
 
+	buf := make([]byte, inboundMTU)
 	for {
 		n, addr, err := p.ReadFrom(buf)
 


Test patch:
diff --git a/internal/server/turn_test.go b/internal/server/turn_test.go
index 86d09e1..c442dbe 100644
--- a/internal/server/turn_test.go
+++ b/internal/server/turn_test.go
@@ -1,36 +1,40 @@
 package server
 
 import (
+	"net"
 	"testing"
 	"time"
 
+	"github.com/pion/logging"
 	"github.com/pion/stun"
+	"github.com/pion/turn/v2/internal/allocation"
 	"github.com/pion/turn/v2/internal/proto"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestAllocationLifeTime(t *testing.T) {
-	lifetime := proto.Lifetime{
-		Duration: 5 * time.Second,
-	}
+	t.Run("Parsing", func(t *testing.T) {
+		lifetime := proto.Lifetime{
+			Duration: 5 * time.Second,
+		}
 
-	m := &stun.Message{}
-	lifetimeDuration := allocationLifeTime(m)
+		m := &stun.Message{}
+		lifetimeDuration := allocationLifeTime(m)
 
-	if lifetimeDuration != proto.DefaultLifetime {
-		t.Errorf("Allocation lifetime should be default time duration")
-	}
+		if lifetimeDuration != proto.DefaultLifetime {
+			t.Errorf("Allocation lifetime should be default time duration")
+		}
 
-	if err := lifetime.AddTo(m); err != nil {
-		t.Errorf("Lifetime add to message failed: %v", err)
-	}
+		assert.NoError(t, lifetime.AddTo(m))
 
-	lifetimeDuration = allocationLifeTime(m)
-	if lifetimeDuration != lifetime.Duration {
-		t.Errorf("Expect lifetimeDuration is %s, but %s", lifetime.Duration, lifetimeDuration)
-	}
+		lifetimeDuration = allocationLifeTime(m)
+		if lifetimeDuration != lifetime.Duration {
+			t.Errorf("Expect lifetimeDuration is %s, but %s", lifetime.Duration, lifetimeDuration)
+		}
+	})
 
 	// If lifetime is bigger than maximumLifetime
-	{
+	t.Run("Overflow", func(t *testing.T) {
 		lifetime := proto.Lifetime{
 			Duration: maximumAllocationLifetime * 2,
 		}
@@ -42,5 +46,59 @@ func TestAllocationLifeTime(t *testing.T) {
 		if lifetimeDuration != proto.DefaultLifetime {
 			t.Errorf("Expect lifetimeDuration is %s, but %s", proto.DefaultLifetime, lifetimeDuration)
 		}
-	}
+	})
+
+	t.Run("DeletionZeroLifetime", func(t *testing.T) {
+		l, err := net.ListenPacket("udp4", "0.0.0.0:0")
+		assert.NoError(t, err)
+		defer func() {
+			assert.NoError(t, l.Close())
+		}()
+
+		logger := logging.NewDefaultLoggerFactory().NewLogger("turn")
+
+		allocationManager, err := allocation.NewManager(allocation.ManagerConfig{
+			AllocatePacketConn: func(network string, requestedPort int) (net.PacketConn, net.Addr, error) {
+				conn, listenErr := net.ListenPacket(network, "0.0.0.0:0")
+				if err != nil {
+					return nil, nil, listenErr
+				}
+
+				return conn, conn.LocalAddr(), nil
+			},
+			AllocateConn: func(network string, requestedPort int) (net.Conn, net.Addr, error) {
+				return nil, nil, nil
+			},
+			LeveledLogger: logger,
+		})
+		assert.NoError(t, err)
+
+		staticKey := []byte("ABC")
+		r := Request{
+			AllocationManager: allocationManager,
+			Conn:              l,
+			SrcAddr:           &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 5000},
+			Log:               logger,
+			AuthHandler: func(username string, realm string, srcAddr net.Addr) (key []byte, ok bool) {
+				return staticKey, true
+			},
+		}
+
+		fiveTuple := &allocation.FiveTuple{SrcAddr: r.SrcAddr, DstAddr: r.Conn.LocalAddr(), Protocol: allocation.UDP}
+
+		_, err = r.AllocationManager.CreateAllocation(fiveTuple, r.Conn, 0, time.Hour)
+		assert.NoError(t, err)
+
+		assert.NotNil(t, r.AllocationManager.GetAllocation(fiveTuple))
+
+		m := &stun.Message{}
+		assert.NoError(t, (proto.Lifetime{}).AddTo(m))
+		assert.NoError(t, (stun.MessageIntegrity(staticKey)).AddTo(m))
+		assert.NoError(t, (stun.Nonce(staticKey)).AddTo(m))
+		assert.NoError(t, (stun.Realm(staticKey)).AddTo(m))
+		assert.NoError(t, (stun.Username(staticKey)).AddTo(m))
+
+		assert.NoError(t, handleRefreshRequest(r, m))
+		assert.Nil(t, r.AllocationManager.GetAllocation(fiveTuple))
+	})
 }
diff --git a/server_test.go b/server_test.go
index 56ef654..d77b72d 100644
--- a/server_test.go
+++ b/server_test.go
@@ -71,28 +71,16 @@ func TestServer(t *testing.T) {
 			Conn:          conn,
 			LoggerFactory: loggerFactory,
 		})
-		if !assert.NoError(t, err, "should succeed") {
-			return
-		}
-		err = client.Listen()
-		if !assert.NoError(t, err, "should succeed") {
-			return
-		}
-		defer func() {
-			assert.NoError(t, conn.Close())
-			client.Close()
-		}()
-
-		log.Debug("sending a binding request.")
+		assert.NoError(t, err)
+		assert.NoError(t, client.Listen())
 
 		_, err = client.SendBindingRequestTo(&net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 3478})
 		assert.NoError(t, err, "should succeed")
 
-		log.Debug("now closing the server...")
+		client.Close()
+		assert.NoError(t, conn.Close())
 
-		// Close server
-		err = server.Close()
-		assert.NoError(t, err, "should succeed")
+		assert.NoError(t, server.Close())
 	})
 }