allegro / allegro/bigcache

Issue with BytesQueue.

Ouverte
#262 1 commentaire 1 réaction 0 personnes assignées Voir sur GitHub
bug
Langage dominant
Go
Étoiles
8.2k
Forks
614
Merge moyen
5 j 12 h
PR mergées (30 j)
1

Description

I was going through the `bytes_queue.go` and have found some inconsistencies with the result. Below is a way to reproduce this issue :

There is test in `bytes_queue_test.go` :

```go
func TestUnchangedEntriesIndexesAfterAdditionalMemoryAllocationWhereTailIsBeforeHead(t *testing.T) {
t.Parallel()

// given
queue := NewBytesQueue(100, 0, false)

// when
queue.Push(blob('a', 70)) // header + entry + left margin = 72 bytes
index, _ := queue.Push(blob('b', 10)) // 72 + 10 + 1 = 83 bytes
queue.Pop() // space freed at the beginning
queue.Push(blob('c', 30)) // 31 bytes used at the beginning, tail pointer is before head pointer
newestIndex, _ := queue.Push(blob('d', 40)) // 41 bytes needed but no available in one segment, allocate new memory

// then
assertEqual(t, 200, queue.Capacity())
assertEqual(t, blob('b', 10), get(queue, index))
assertEqual(t, blob('d', 40), get(queue, newestIndex))
}
```

Now lets change this test and bit and add the following line :

```go
func TestUnchangedEntriesIndexesAfterAdditionalMemoryAllocationWhereTailIsBeforeHead(t *testing.T) {
t.Parallel()

// given
queue := NewBytesQueue(100, 0, false)

// when
queue.Push(blob('a', 70)) // header + entry + left margin = 72 bytes
index, _ := queue.Push(blob('b', 10)) // 72 + 10 + 1 = 83 bytes
queue.Pop() // space freed at the beginning
queue.Push(blob('c', 30)) // 31 bytes used at the beginning, tail pointer is before head pointer
newestIndex, _ := queue.Push(blob('d', 40)) // 41 bytes needed but no available in one segment, allocate new memory

// then
assertEqual(t, string(blob('b', 10)), string(pop(queue))) // THIS LINE ADDED
assertEqual(t, 200, queue.Capacity())
assertEqual(t, blob('b', 10), get(queue, index))
assertEqual(t, blob('d', 40), get(queue, newestIndex))
}
```

![Screen Shot 2021-01-17 at 12 04 20 PM](https://user-images.githubusercontent.com/77553735/104854553-230bbd80-58bc-11eb-8a87-15e09754212a.png)

As seen in the above screen shot, the expected string should be `blob('b',10)` and not `blob('c', 30)`.

I have pointed the bug happening in the `allocateAdditionalMemory` :

```go
if leftMarginIndex != q.rightMargin {
copy(q.array, oldArray[:q.rightMargin])

if q.tail <= q.head {
if q.tail != q.head {
headerEntrySize := getUvarintSize(uint32(q.head - q.tail))
emptyBlobLen := q.head - q.tail - headerEntrySize
q.push(make([]byte, emptyBlobLen), emptyBlobLen)
}

q.head = leftMarginIndex // Why are we doing this ?
q.tail = q.rightMargin
}
}
```

Ideally we should not be assigning `q.head = leftMarginIndex` this way you are actually breaking FIFO rules.

Guide de contribution

Aucun guide de contribution indexé pour ce dépôt

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.