Skip to content

Commit

Permalink
transport: don't reuse resChan, fixed a race condition
Browse files Browse the repository at this point in the history
  • Loading branch information
IrineSistiana committed Oct 27, 2023
1 parent 5caa4ac commit 0f784a8
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 24 deletions.
8 changes: 3 additions & 5 deletions pkg/upstream/transport/conn_traditional.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (dc *TraditionalDnsConn) exchange(ctx context.Context, q []byte) (*[]byte,
if respChan == nil {
return nil, ErrTDCTooManyQueries
}
defer dc.deleteQueueC(assignedQid, respChan)
defer dc.deleteQueueC(assignedQid)

// Reminder: Set write deadline here is not very useful to avoid dead connections.
// Typically, a write operation will time out only if its socket buffer is full.
Expand Down Expand Up @@ -229,7 +229,7 @@ func (dc *TraditionalDnsConn) queueLen() int {
// It returns a nil c if queue has too many queries.
// Caller must call deleteQueueC to release the qid in queue.
func (dc *TraditionalDnsConn) addQueueC() (qid uint16, c chan *[]byte) {
c = getRespChan()
c = make(chan *[]byte)
dc.queueMu.Lock()
for i := 0; i < 100; i++ {
qid = dc.nextQid
Expand All @@ -244,15 +244,13 @@ func (dc *TraditionalDnsConn) addQueueC() (qid uint16, c chan *[]byte) {
dc.queueMu.Unlock()

// Too many queries in queue. Can't assign qid.
releaseRespChan(c)
return 0, nil
}

func (dc *TraditionalDnsConn) deleteQueueC(qid uint16, c chan *[]byte) {
func (dc *TraditionalDnsConn) deleteQueueC(qid uint16) {
dc.queueMu.Lock()
delete(dc.queue, uint32(qid))
dc.queueMu.Unlock()
releaseRespChan(c)
}

func (dc *TraditionalDnsConn) ReserveNewQuery() (_ ReservedExchanger, closed bool) {
Expand Down
19 changes: 0 additions & 19 deletions pkg/upstream/transport/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package transport
import (
"encoding/binary"
"io"
"sync"

"github.com/IrineSistiana/mosdns/v5/pkg/pool"
"github.com/miekg/dns"
Expand Down Expand Up @@ -51,24 +50,6 @@ func copyMsg(m []byte) *[]byte {
return bp
}

var respChanPool = sync.Pool{
New: func() any {
return make(chan *[]byte, 1)
},
}

func getRespChan() chan *[]byte {
return respChanPool.Get().(chan *[]byte)
}
func releaseRespChan(c chan *[]byte) {
select {
case payload := <-c:
ReleaseResp(payload)
default:
}
respChanPool.Put(c)
}

// readMsgUdp reads dns frame from r. r typically should be a udp connection.
// It uses a 4kb rx buffer and ignores any payload that is too small for a dns msg.
// If no error, the length of payload always >= 12 bytes.
Expand Down

0 comments on commit 0f784a8

Please sign in to comment.