Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Priority Queue has an infinite Loop bug #278

Open
hafus opened this issue Oct 7, 2024 · 3 comments
Open

Priority Queue has an infinite Loop bug #278

hafus opened this issue Oct 7, 2024 · 3 comments

Comments

@hafus
Copy link

hafus commented Oct 7, 2024

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.

head := q.next
prev := q.next
for head != nil {
if priority <= head.priority {
break
}
prev = head
head = head.next
}
if head == nil {
if prev != nil {
prev.next = newPq
}
newPq.prev = prev
} else {
newPq.next = head
newPq.prev = prev
if prev != nil {
prev.next = newPq
}
head.prev = newPq

suggestions

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

@hafus
Copy link
Author

hafus commented Oct 13, 2024

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

@Sean-Der
Copy link
Member

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 added a commit that referenced this issue Oct 14, 2024
Can be used by NACK and JitterBuffer now

Relates to #278
Sean-Der added a commit that referenced this issue Oct 14, 2024
Can be used by NACK and JitterBuffer now

Relates to #278
@hafus
Copy link
Author

hafus commented Oct 15, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants