Modification from review feedback

Resolves #74

client.go

@@ -307,7 +307,7 @@ func (c *Client) Allocate() (net.PacketConn, error) {
 }
 
 // PerformTransaction performs STUN transaction
-func (c *Client) PerformTransaction(msg *stun.Message, to net.Addr, dontWait bool) (client.TrResult, error) {
+func (c *Client) PerformTransaction(msg *stun.Message, to net.Addr, dontWait bool) (client.TransactionResult, error) {
 	trKey := b64.StdEncoding.EncodeToString(msg.TransactionID[:])
 	tr := client.NewTransaction(&client.TransactionConfig{
 		Key:      trKey,
@@ -321,14 +321,14 @@ func (c *Client) PerformTransaction(msg *stun.Message, to net.Addr, dontWait boo
 	c.log.Tracef("start %s transaction %s to %s", msg.Type, trKey, tr.To.String())
 	_, err := c.conn.WriteTo(msg.Raw, to)
 	if err != nil {
-		return client.TrResult{}, err
+		return client.TransactionResult{}, err
 	}
 
 	tr.StartRtxTimer(c.onRtxTimeout)
 
 	// If dontWait is true, get the transaction going and return immediately
 	if dontWait {
-		return client.TrResult{}, nil
+		return client.TransactionResult{}, nil
 	}
 
 	res := tr.WaitForResult()
@@ -358,11 +358,7 @@ func (c *Client) HandleInbound(data []byte, from net.Addr) (bool, error) {
 		return false, nil // unhandled
 	}
 
-	if err != nil {
-		return true, err
-	}
-
-	return true, nil // handled
+	return true, err
 }
 
 func (c *Client) handleSTUNMessage(data []byte, from net.Addr) error {
@@ -421,7 +417,7 @@ func (c *Client) handleSTUNMessage(data []byte, from net.Addr) error {
 	tr.StopRtxTimer()
 	c.trMap.Delete(trKey)
 
-	if !tr.WriteResult(client.TrResult{Msg: msg, From: from}) {
+	if !tr.WriteResult(client.TransactionResult{Msg: msg, From: from}) {
 		c.log.Debugf("no listener for %s", msg.String())
 	}
 
@@ -442,7 +438,7 @@ func (c *Client) onRtxTimeout(trKey string, nRtx int32) {
 	if nRtx == maxRtxCount {
 		// all retransmisstions failed
 		c.trMap.Delete(trKey)
-		if !tr.WriteResult(client.TrResult{
+		if !tr.WriteResult(client.TransactionResult{
 			Err: fmt.Errorf("all retransmissions for %s failed", trKey),
 		}) {
 			c.log.Debug("no listener for transaction")
@@ -455,7 +451,7 @@ func (c *Client) onRtxTimeout(trKey string, nRtx int32) {
 	_, err := c.conn.WriteTo(tr.Raw, tr.To)
 	if err != nil {
 		c.trMap.Delete(trKey)
-		if !tr.WriteResult(client.TrResult{
+		if !tr.WriteResult(client.TransactionResult{
 			Err: fmt.Errorf("failed to retransmit transaction %s", trKey),
 		}) {
 			c.log.Debug("no listener for transaction")

internal/client/atomic_bool.go

@@ -30,7 +30,7 @@ func (b *AtomicBool) SetToFalse() {
 
 // True returns true if it is set to true
 func (b *AtomicBool) True() bool {
-	return atomic.LoadInt32(&b.n) == int32(1)
+	return atomic.LoadInt32(&b.n) != int32(0)
 }
 
 // False return true if it is set to false

internal/client/conn.go

@@ -38,7 +38,7 @@ type UDPConnObserver interface {
 	Username() stun.Username
 	Realm() stun.Realm
 	WriteTo(data []byte, to net.Addr) (int, error)
-	PerformTransaction(msg *stun.Message, to net.Addr, dontWait bool) (TrResult, error)
+	PerformTransaction(msg *stun.Message, to net.Addr, dontWait bool) (TransactionResult, error)
 }
 
 // UDPConnConfig is a set of configuration params use by NewUDPConn
@@ -127,7 +127,6 @@ func NewUDPConn(config *UDPConnConfig) *UDPConn {
 // an Error with Timeout() == true after a fixed time limit;
 // see SetDeadline and SetReadDeadline.
 func (c *UDPConn) ReadFrom(p []byte) (n int, addr net.Addr, err error) {
-loop:
 	for c.closed.False() {
 		select {
 		case ibData := <-c.readCh:
@@ -147,7 +146,6 @@ loop:
 
 		case <-c.closeCh:
 			c.closed.SetToTrue()
-			break loop
 		}
 	}
 

internal/client/transaction.go

@@ -12,8 +12,8 @@ const (
 	maxRtxInterval time.Duration = 1600 * time.Millisecond
 )
 
-// TrResult is a bag of result values of a transaction
-type TrResult struct {
+// TransactionResult is a bag of result values of a transaction
+type TransactionResult struct {
 	Msg  *stun.Message
 	From net.Addr
 	Err  error
@@ -29,24 +29,24 @@ type TransactionConfig struct {
 
 // Transaction represents a transaction
 type Transaction struct {
-	Key      string        // read-only
-	Raw      []byte        // read-only
-	To       net.Addr      // read-only
-	nRtx     int32         // modified only by the timer thread
-	interval time.Duration // modified only by the timer thread
-	timer    *time.Timer   // therad-safe, set only by the creator, and stopper
-	resultCh chan TrResult // thread-safe
+	Key      string                 // read-only
+	Raw      []byte                 // read-only
+	To       net.Addr               // read-only
+	nRtx     int32                  // modified only by the timer thread
+	interval time.Duration          // modified only by the timer thread
+	timer    *time.Timer            // therad-safe, set only by the creator, and stopper
+	resultCh chan TransactionResult // thread-safe
 	mutex    sync.RWMutex
 }
 
 // NewTransaction creates a new instance of Transaction
 func NewTransaction(config *TransactionConfig) *Transaction {
 	return &Transaction{
-		Key:      config.Key,          // read-only
-		Raw:      config.Raw,          // read-only
-		To:       config.To,           // read-only
-		interval: config.Interval,     // modified only by the timer thread
-		resultCh: make(chan TrResult), // thread-safe
+		Key:      config.Key,                   // read-only
+		Raw:      config.Raw,                   // read-only
+		To:       config.To,                    // read-only
+		interval: config.Interval,              // modified only by the timer thread
+		resultCh: make(chan TransactionResult), // thread-safe
 	}
 }
 
@@ -76,7 +76,7 @@ func (t *Transaction) StopRtxTimer() {
 }
 
 // WriteResult writes the result to the result channel
-func (t *Transaction) WriteResult(res TrResult) bool {
+func (t *Transaction) WriteResult(res TransactionResult) bool {
 	select {
 	case t.resultCh <- res:
 		return true
@@ -86,7 +86,7 @@ func (t *Transaction) WriteResult(res TrResult) bool {
 }
 
 // WaitForResult waits for the transaction result
-func (t *Transaction) WaitForResult() TrResult {
+func (t *Transaction) WaitForResult() TransactionResult {
 	return <-t.resultCh
 }
 

internal/client/trylock.go

@@ -21,8 +21,5 @@ func (c *TryLock) Lock() error {
 
 // Unlock unlocks the try-lock.
 func (c *TryLock) Unlock() {
-	if !atomic.CompareAndSwapInt32(&c.n, 1, 0) {
-		// this should never ever happen, so panic!
-		panic(fmt.Errorf("try-lock was not locked"))
-	}
+	atomic.StoreInt32(&c.n, 0)
 }