Skip to content

Commit

Permalink
logtail: require Buffer.Write to not retain the provided slice (#11617)
Browse files Browse the repository at this point in the history
Buffer.Write has the exact same signature of io.Writer.Write.
The latter requires that implementations to never retain
the provided input buffer, which is an expectation that most
users will have when they see a Write signature.

The current behavior of Buffer.Write where it does retain
the input buffer is a risky precedent to set.
Switch the behavior to match io.Writer.Write.

There are only two implementations of Buffer in existence:
* logtail.memBuffer
* filch.Filch

The former can be fixed by cloning the input to Write.
This will cause an extra allocation in every Write,
but we can fix that will pooling on the caller side
in a follow-up PR.

The latter only passes the input to os.File.Write,
which does respect the io.Writer.Write requirements.

Updates #cleanup
Updates tailscale/corp#18514

Signed-off-by: Joe Tsai <joetsai@digital-static.net>
  • Loading branch information
dsnet committed Apr 8, 2024
1 parent 231e44e commit b4ba492
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions logtail/buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package logtail

import (
"bytes"
"errors"
"fmt"
"sync"
Expand All @@ -19,8 +20,7 @@ type Buffer interface {
TryReadLine() ([]byte, error)

// Write writes a log line into the ring buffer.
//
// Write takes ownership of the provided slice.
// Implementations must not retain the provided buffer.
Write([]byte) (int, error)
}

Expand Down Expand Up @@ -62,7 +62,7 @@ func (m *memBuffer) Write(b []byte) (int, error) {
defer m.dropMu.Unlock()

ent := qentry{
msg: b,
msg: bytes.Clone(b),
dropCount: m.dropCount,
}
select {
Expand Down

0 comments on commit b4ba492

Please sign in to comment.