From b4ba492701a5da24fa8f4b19bc9651ec882506bc Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 8 Apr 2024 15:01:07 -0700 Subject: [PATCH] logtail: require Buffer.Write to not retain the provided slice (#11617) 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 --- logtail/buffer.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/logtail/buffer.go b/logtail/buffer.go index dbb0552f1..c9f2e1ad0 100644 --- a/logtail/buffer.go +++ b/logtail/buffer.go @@ -4,6 +4,7 @@ package logtail import ( + "bytes" "errors" "fmt" "sync" @@ -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) } @@ -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 {