interceptor icon indicating copy to clipboard operation
interceptor copied to clipboard

Priority Queue has an infinite Loop bug

Open hafus opened this issue 1 year ago • 3 comments

Your environment.

  • Version: v0.1.36
  • Browser: no browser

What did you do?

The code snippet below is a test case that catches the reported bug.

t.Run("Infinite loop", func(*testing.T) {
		q := NewQueue()
		pkt := &rtp.Packet{Header: rtp.Header{SequenceNumber: 5000, Timestamp: 500}, Payload: []byte{0x02}}
		q.Push(pkt, pkt.SequenceNumber)
		q.Push(pkt, pkt.SequenceNumber)
		assert.Equal(uint16(1), q.Length())
		popped, _ := q.PopAt(uint16(5012))
		assert.Equal(popped.SequenceNumber, uint16(5000))
		assert.Equal(uint16(0), q.Length())

	})

What did you expect?

Priority queue should have a length of one after inserting a duplicate packet of the first packet in the queue or at least not enter into an infinite loop.

What happened?

The Push func runs for infinite time because the duplicated packet has head and prev pointers pointing into the head packet, resulting in a loop. https://github.com/pion/interceptor/blob/0eab1887cec79081e21457c0f9f89be2537c5df5/pkg/jitterbuffer/priority_queue.go#L80-L100

suggestions

I suggest dropping any duplicated packet because it already exists in the queue.

hafus avatar Oct 07 '24 20:10 hafus

@Sean-Der, please have a look at the issue above.

hafus avatar Oct 13 '24 20:10 hafus

Hey @hafus I would really appreciate your help on this.

I want to drop the Priority Queue and instead have the JitterBuffer and NACK Responder powered by the same data structure. Would you be interested in helping me with this?

Sean-Der avatar Oct 14 '24 02:10 Sean-Der

Hello @Sean-Der

Of course, that's my pleasure. I will start working on it. If you have any suggestions or known issues about the current implementation, please share them with me.

hafus avatar Oct 15 '24 22:10 hafus