allegro / allegro/bigcache

Issue with BytesQueue.

Abierto
#262 1 comentario 1 reacción 0 asignados Ver en GitHub
bug
Lenguaje dominante
Go
Estrellas
8.2k
Forks
614
Merge medio
5 d 12 h
PR fusionados (30 d)
1

Descripción

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.

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.