Repository: pion/ice
Pull request #259 (2020-08-05 06:54:14)

fix nogo warnings on logs
#### Description

#### Reference issue
Fixes #...


Sean-Der: @nerd2 LGTM! Thanks for the PR. Would you mind fixing the commit message

`bash .github/lint-commit-message.sh` to run the check locally. I think you just need subject+body.
codecov[bot]: # [Codecov](https://codecov.io/gh/pion/ice/pull/259?src=pr&el=h1) Report
> Merging [#259](https://codecov.io/gh/pion/ice/pull/259?src=pr&el=desc) into [master](https://codecov.io/gh/pion/ice/commit/8f4d021fc7e54cbeb9ac5633c80ef19b92e1a276&el=desc) will **decrease** coverage by `0.16%`.
> The diff coverage is `0.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pion/ice/pull/259/graphs/tree.svg?width=650&height=150&src=pr&token=1ivIyHDzva)](https://codecov.io/gh/pion/ice/pull/259?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #259      +/-   ##
==========================================
- Coverage   80.28%   80.12%   -0.17%     
==========================================
  Files          31       31              
  Lines        2450     2450              
==========================================
- Hits         1967     1963       -4     
- Misses        344      348       +4     
  Partials      139      139              
```

| Flag | Coverage Δ | |
|---|---|---|
| #go | `80.12% <0.00%> (-0.17%)` | :arrow_down: |
| #wasm | `23.18% <0.00%> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/ice/pull/259?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [gather.go](https://codecov.io/gh/pion/ice/pull/259/diff?src=pr&el=tree#diff-Z2F0aGVyLmdv) | `70.58% <0.00%> (-1.24%)` | :arrow_down: |
| [tcp\_mux.go](https://codecov.io/gh/pion/ice/pull/259/diff?src=pr&el=tree#diff-dGNwX211eC5nbw==) | `76.25% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/ice/pull/259?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/ice/pull/259?src=pr&el=footer). Last update [8f4d021...a8ae973](https://codecov.io/gh/pion/ice/pull/259?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/.github/workflows/tidy-check.yaml b/.github/workflows/tidy-check.yaml
new file mode 100644
index 0000000..03b5189
--- /dev/null
+++ b/.github/workflows/tidy-check.yaml
@@ -0,0 +1,37 @@
+#
+# DO NOT EDIT THIS FILE
+#
+# It is automatically copied from https://github.com/pion/.goassets repository.
+# If this repository should have package specific CI config,
+# remove the repository name from .goassets/.github/workflows/assets-sync.yml.
+#
+# If you want to update the shared CI config, send a PR to
+# https://github.com/pion/.goassets instead of this repository.
+#
+
+name: Go mod tidy
+on:
+  pull_request:
+    branches:
+      - master
+  push:
+    branches:
+      - master
+
+jobs:
+  Check:
+    runs-on: ubuntu-latest
+    steps:
+      - name: checkout
+        uses: actions/checkout@v2
+      - name: Setup Go
+        uses: actions/setup-go@v2
+      - name: check
+        run: |
+          go mod download
+          go mod tidy
+          if ! git diff --exit-code
+          then
+            echo "Not go mod tidied"
+            exit 1
+          fi
diff --git a/.travis.yml b/.travis.yml
index 9bd21a3..43eb708 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -113,30 +113,30 @@ _test_wasm_job: &test_wasm_job
 jobs:
   include:
     - <<: *lint_job
-      name: Lint 1.14
-      go: 1.14
-    - <<: *test_job
-      name: Test 1.13
-      go: 1.13
+      name: Lint 1.15
+      go: 1.15
     - <<: *test_job
       name: Test 1.14
       go: 1.14
-    - <<: *test_i386_job
-      name: Test i386 1.13
-      env: GO_VERSION=1.13
-      go: 1.14  # Go version for host environment only for `go list`.
-                # All tests are done on the version specified by GO_VERSION.
+    - <<: *test_job
+      name: Test 1.15
+      go: 1.15
     - <<: *test_i386_job
       name: Test i386 1.14
       env: GO_VERSION=1.14
-      go: 1.14  # Go version for host environment only for `go list`.
+      go: 1.15  # Go version for host environment only for `go list`.
+                # All tests are done on the version specified by GO_VERSION.
+    - <<: *test_i386_job
+      name: Test i386 1.15
+      env: GO_VERSION=1.15
+      go: 1.15  # Go version for host environment only for `go list`.
                 # All tests are done on the version specified by GO_VERSION.
-    - <<: *test_wasm_job
-      name: Test WASM 1.13
-      env: GO_VERSION=1.13
     - <<: *test_wasm_job
       name: Test WASM 1.14
       env: GO_VERSION=1.14
+    - <<: *test_wasm_job
+      name: Test WASM 1.15
+      env: GO_VERSION=1.15
 
 notifications:
   email: false
diff --git a/README.md b/README.md
index 4e76e5c..0daf743 100644
--- a/README.md
+++ b/README.md
@@ -55,6 +55,8 @@ Check out the **[contributing wiki](https://github.com/pion/webrtc/wiki/Contribu
 * [BUPTCZQ](https://github.com/buptczq)
 * [Henry](https://github.com/cryptix)
 * [Jerko Steiner](https://github.com/jeremija)
+* [Sidney San Martín](https://github.com/s4y)
+* [JooYoung Lim](https://github.com/DevRockstarZ)
 
 ### License
 MIT License - see [LICENSE](LICENSE) for full text
diff --git a/agent.go b/agent.go
index 26ed93f..a81db93 100644
--- a/agent.go
+++ b/agent.go
@@ -692,7 +692,8 @@ func (a *Agent) checkKeepalive() {
 	}
 
 	if (a.keepaliveInterval != 0) &&
-		(time.Since(selectedPair.local.LastSent()) > a.keepaliveInterval) {
+		((time.Since(selectedPair.local.LastSent()) > a.keepaliveInterval) ||
+			(time.Since(selectedPair.remote.LastReceived()) > a.keepaliveInterval)) {
 		// we use binding request instead of indication to support refresh consent schemas
 		// see https://tools.ietf.org/html/rfc7675
 		a.selector.PingCandidate(selectedPair.local, selectedPair.remote)
@@ -741,7 +742,7 @@ func (a *Agent) resolveAndAddMulticastCandidate(c *CandidateHost) {
 	if a.mDNSConn == nil {
 		return
 	}
-	_, src, err := a.mDNSConn.Query(context.TODO(), c.Address())
+	_, src, err := a.mDNSConn.Query(c.context(), c.Address())
 	if err != nil {
 		a.log.Warnf("Failed to discover mDNS candidate %s: %v", c.Address(), err)
 		return
diff --git a/candidate_base.go b/candidate_base.go
index 12313cd..59377f0 100644
--- a/candidate_base.go
+++ b/candidate_base.go
@@ -90,6 +90,72 @@ func (c *candidateBase) Component() uint16 {
 
 // LocalPreference returns the local preference for this candidate
 func (c *candidateBase) LocalPreference() uint16 {
+	if c.NetworkType().IsTCP() {
+		// RFC 6544, section 4.2
+		//
+		// In Section 4.1.2.1 of [RFC5245], a recommended formula for UDP ICE
+		// candidate prioritization is defined.  For TCP candidates, the same
+		// formula and candidate type preferences SHOULD be used, and the
+		// RECOMMENDED type preferences for the new candidate types defined in
+		// this document (see Section 5) are 105 for NAT-assisted candidates and
+		// 75 for UDP-tunneled candidates.
+		//
+		// (...)
+		//
+		// With TCP candidates, the local preference part of the recommended
+		// priority formula is updated to also include the directionality
+		// (active, passive, or simultaneous-open) of the TCP connection.  The
+		// RECOMMENDED local preference is then defined as:
+		//
+		//     local preference = (2^13) * direction-pref + other-pref
+		//
+		// The direction-pref MUST be between 0 and 7 (both inclusive), with 7
+		// being the most preferred.  The other-pref MUST be between 0 and 8191
+		// (both inclusive), with 8191 being the most preferred.  It is
+		// RECOMMENDED that the host, UDP-tunneled, and relayed TCP candidates
+		// have the direction-pref assigned as follows: 6 for active, 4 for
+		// passive, and 2 for S-O.  For the NAT-assisted and server reflexive
+		// candidates, the RECOMMENDED values are: 6 for S-O, 4 for active, and
+		// 2 for passive.
+		//
+		// (...)
+		//
+		// If any two candidates have the same type-preference and direction-
+		// pref, they MUST have a unique other-pref.  With this specification,
+		// this usually only happens with multi-homed hosts, in which case
+		// other-pref is the preference for the particular IP address from which
+		// the candidate was obtained.  When there is only a single IP address,
+		// this value SHOULD be set to the maximum allowed value (8191).
+		var otherPref uint16 = 8191
+
+		directionPref := func() uint16 {
+			switch c.Type() {
+			case CandidateTypeHost, CandidateTypeRelay:
+				switch c.tcpType {
+				case TCPTypeActive:
+					return 6
+				case TCPTypePassive:
+					return 4
+				case TCPTypeSimultaneousOpen:
+					return 2
+				}
+			case CandidateTypePeerReflexive, CandidateTypeServerReflexive:
+				switch c.tcpType {
+				case TCPTypeSimultaneousOpen:
+					return 6
+				case TCPTypeActive:
+					return 4
+				case TCPTypePassive:
+					return 2
+				}
+			}
+
+			return 0
+		}()
+
+		return (1<<13)*directionPref + otherPref
+	}
+
 	return defaultLocalPreference
 }
 
diff --git a/gather.go b/gather.go
index 7d6a7ba..d081cf5 100644
--- a/gather.go
+++ b/gather.go
@@ -169,7 +169,7 @@ func (a *Agent) gatherCandidatesLocal(ctx context.Context, networkTypes []Networ
 				conn, err = a.tcpMux.GetConnByUfrag(a.localUfrag)
 				if err != nil {
 					if err != ErrTCPMuxNotInitialized {
-						a.log.Warnf("error getting tcp conn by ufrag: %s %s\n", network, ip, a.localUfrag)
+						a.log.Warnf("error getting tcp conn by ufrag: %s %s %s\n", network, ip, a.localUfrag)
 					}
 					continue
 				}
diff --git a/tcp_mux.go b/tcp_mux.go
index 5bad617..3bddf63 100644
--- a/tcp_mux.go
+++ b/tcp_mux.go
@@ -208,7 +208,7 @@ func (m *TCPMuxDefault) handleConn(conn net.Conn) {
 
 	if err := packetConn.AddConn(conn, buf); err != nil {
 		m.closeAndLogError(conn)
-		m.params.Logger.Warnf("Error adding conn to tcpPacketConn from %s to %s, %w\n", conn.RemoteAddr(), conn.LocalAddr(), err)
+		m.params.Logger.Warnf("Error adding conn to tcpPacketConn from %s to %s: %s\n", conn.RemoteAddr(), conn.LocalAddr(), err)
 		return
 	}
 }
diff --git a/tcp_packet_conn.go b/tcp_packet_conn.go
index 99a462d..264dea4 100644
--- a/tcp_packet_conn.go
+++ b/tcp_packet_conn.go
@@ -194,9 +194,10 @@ func (t *tcpPacketConn) removeConn(conn net.Conn) {
 func (t *tcpPacketConn) Close() error {
 	t.mu.Lock()
 
+	var shouldCloseRecvChan bool
 	t.closeOnce.Do(func() {
 		close(t.closedChan)
-		close(t.recvChan)
+		shouldCloseRecvChan = true
 	})
 
 	for _, conn := range t.conns {
@@ -208,6 +209,10 @@ func (t *tcpPacketConn) Close() error {
 
 	t.wg.Wait()
 
+	if shouldCloseRecvChan {
+		close(t.recvChan)
+	}
+
 	return nil
 }
 


Test patch:
diff --git a/candidate_test.go b/candidate_test.go
index e3d41f6..fea3492 100644
--- a/candidate_test.go
+++ b/candidate_test.go
@@ -21,6 +21,39 @@ func TestCandidatePriority(t *testing.T) {
 			},
 			WantPriority: 2130706431,
 		},
+		{
+			Candidate: &CandidateHost{
+				candidateBase: candidateBase{
+					candidateType: CandidateTypeHost,
+					component:     ComponentRTP,
+					networkType:   NetworkTypeTCP4,
+					tcpType:       TCPTypeActive,
+				},
+			},
+			WantPriority: 2128609279,
+		},
+		{
+			Candidate: &CandidateHost{
+				candidateBase: candidateBase{
+					candidateType: CandidateTypeHost,
+					component:     ComponentRTP,
+					networkType:   NetworkTypeTCP4,
+					tcpType:       TCPTypePassive,
+				},
+			},
+			WantPriority: 2124414975,
+		},
+		{
+			Candidate: &CandidateHost{
+				candidateBase: candidateBase{
+					candidateType: CandidateTypeHost,
+					component:     ComponentRTP,
+					networkType:   NetworkTypeTCP4,
+					tcpType:       TCPTypeSimultaneousOpen,
+				},
+			},
+			WantPriority: 2120220671,
+		},
 		{
 			Candidate: &CandidatePeerReflexive{
 				candidateBase: candidateBase{
@@ -30,6 +63,39 @@ func TestCandidatePriority(t *testing.T) {
 			},
 			WantPriority: 1862270975,
 		},
+		{
+			Candidate: &CandidatePeerReflexive{
+				candidateBase: candidateBase{
+					candidateType: CandidateTypePeerReflexive,
+					component:     ComponentRTP,
+					networkType:   NetworkTypeTCP6,
+					tcpType:       TCPTypeSimultaneousOpen,
+				},
+			},
+			WantPriority: 1860173823,
+		},
+		{
+			Candidate: &CandidatePeerReflexive{
+				candidateBase: candidateBase{
+					candidateType: CandidateTypePeerReflexive,
+					component:     ComponentRTP,
+					networkType:   NetworkTypeTCP6,
+					tcpType:       TCPTypeActive,
+				},
+			},
+			WantPriority: 1855979519,
+		},
+		{
+			Candidate: &CandidatePeerReflexive{
+				candidateBase: candidateBase{
+					candidateType: CandidateTypePeerReflexive,
+					component:     ComponentRTP,
+					networkType:   NetworkTypeTCP6,
+					tcpType:       TCPTypePassive,
+				},
+			},
+			WantPriority: 1851785215,
+		},
 		{
 			Candidate: &CandidateServerReflexive{
 				candidateBase: candidateBase{