Files
go-kv/wal/writer_test.go
dailz 408138b3c8 fix: prevent send-on-closed-channel panic in concurrent Put/Delete + Close (C7)
Put/Delete checked writeStopped then called queue.Submit (which sends on
a channel). Close set writeStopped then closed the channel. With no
synchronization between the check and Submit, a concurrent Close could
close the channel during the window, causing "send on closed channel"
panic in Put/Delete.

The CommitQueue.Close comment said "Callers must stop submitting before
Close" but Put/Delete didn't enforce this.

Changes:
- wal/writer.go: add submitMu sync.RWMutex. Put/Delete construct entry
  outside the lock, then hold RLock during writeStopped check + Submit;
  release before <-req.Result. Close holds write lock during
  writeStopped.Store + queue.Close, guaranteeing no Submit is in progress
  when the channel is closed.
- wal/writer_test.go: add TestConcurrentPutCloseNoPanic and
  TestConcurrentDeleteCloseNoPanic. 100 goroutines + 1 closer, recover
  panics. Run with -count=50 for regression detection.

Verified: all existing tests pass. New concurrent tests pass with
-count=50 (0 panics). go test -race ./... green.

Audit context: docs/audit-3.2.md C7.
2026-06-18 13:25:30 +08:00

266 lines
5.8 KiB
Go

package wal
import (
"errors"
"fmt"
"sync"
"sync/atomic"
"testing"
"github.com/dailz/go-kv/config"
"github.com/dailz/go-kv/errkit"
)
func newTestWalWriter(t *testing.T) *WalWriter {
t.Helper()
cfg := config.Defaults()
ww, err := NewWalWriter(&cfg, t.TempDir(), 0, 0)
if err != nil {
t.Fatalf("NewWalWriter: %v", err)
}
t.Cleanup(func() {
if err := ww.Close(); err != nil {
t.Fatalf("Close: %v", err)
}
})
return ww
}
func TestWalWriterSinglePut(t *testing.T) {
ww := newTestWalWriter(t)
if err := ww.Put([]byte("k1"), []byte("v1")); err != nil {
t.Fatalf("Put: %v", err)
}
got := ww.Get([]byte("k1"))
if !got.Found {
t.Fatal("Get(k1) not found")
}
if string(got.Value) != "v1" {
t.Fatalf("Get(k1) value = %q, want %q", got.Value, "v1")
}
}
func TestWalWriterMultiplePuts(t *testing.T) {
ww := newTestWalWriter(t)
for i := range 10 {
key := fmt.Appendf(nil, "k%d", i)
value := fmt.Appendf(nil, "v%d", i)
if err := ww.Put(key, value); err != nil {
t.Fatalf("Put(%q): %v", key, err)
}
}
for i := range 10 {
key := fmt.Appendf(nil, "k%d", i)
want := fmt.Sprintf("v%d", i)
got := ww.Get(key)
if !got.Found {
t.Fatalf("Get(%q) not found", key)
}
if string(got.Value) != want {
t.Fatalf("Get(%q) value = %q, want %q", key, got.Value, want)
}
}
}
func TestWalWriterGroupCommit(t *testing.T) {
ww := newTestWalWriter(t)
const writers = 5
var wg sync.WaitGroup
errCh := make(chan error, writers)
for i := range writers {
wg.Go(func() {
key := fmt.Appendf(nil, "group-k%d", i)
value := fmt.Appendf(nil, "group-v%d", i)
if err := ww.Put(key, value); err != nil {
errCh <- fmt.Errorf("put %d: %w", i, err)
}
})
}
wg.Wait()
close(errCh)
for err := range errCh {
if err != nil {
t.Fatal(err)
}
}
for i := range writers {
key := fmt.Appendf(nil, "group-k%d", i)
want := fmt.Sprintf("group-v%d", i)
got := ww.Get(key)
if !got.Found {
t.Fatalf("Get(%q) not found", key)
}
if string(got.Value) != want {
t.Fatalf("Get(%q) value = %q, want %q", key, got.Value, want)
}
}
}
func TestWalWriterDelete(t *testing.T) {
ww := newTestWalWriter(t)
if err := ww.Put([]byte("k"), []byte("v")); err != nil {
t.Fatalf("Put: %v", err)
}
if err := ww.Delete([]byte("k")); err != nil {
t.Fatalf("Delete: %v", err)
}
got := ww.Get([]byte("k"))
if got.Found {
t.Fatalf("Get(k) found deleted key with value %q", got.Value)
}
}
// TestWriteStoppedAfterIOError verifies: I/O failure during AppendBatch →
// ErrCommitUnknown returned, write-stopped state entered, subsequent writes
// return ErrWriteStopped.
func TestWriteStoppedAfterIOError(t *testing.T) {
cfg := config.Defaults()
ww, err := NewWalWriter(&cfg, t.TempDir(), 0, 0)
if err != nil {
t.Fatalf("NewWalWriter: %v", err)
}
if err := ww.Put([]byte("ok-key"), []byte("ok-val")); err != nil {
t.Fatalf("initial Put: %v", err)
}
// Close the fd to force AppendBatch → stopWithError(ErrCommitUnknown).
if err := ww.segManager.active.fd.Close(); err != nil {
t.Fatalf("close fd: %v", err)
}
err = ww.Put([]byte("fail-key"), []byte("fail-val"))
if err == nil {
t.Fatal("expected error from Put after I/O failure, got nil")
}
if !errors.Is(err, errkit.ErrCommitUnknown) {
t.Errorf("Put error = %v, want ErrCommitUnknown", err)
}
if !ww.IsWriteStopped() {
t.Fatal("IsWriteStopped() = false, want true after I/O error")
}
err = ww.Put([]byte("after-key"), []byte("after-val"))
if !errors.Is(err, errkit.ErrWriteStopped) {
t.Errorf("Put after write-stopped error = %v, want ErrWriteStopped", err)
}
err = ww.Delete([]byte("after-key"))
if !errors.Is(err, errkit.ErrWriteStopped) {
t.Errorf("Delete after write-stopped error = %v, want ErrWriteStopped", err)
}
_ = ww.Close()
}
// -------- C7 regression guards --------
// Regression guard for C7: concurrent Put + Close must not panic.
// Buggy code has a race window between writeStopped check and Submit's
// send on channel; Close closing the channel during that window causes
// "send on closed channel" panic.
//
// Note: this test guarantees "no panic AFTER fix", not "must panic BEFORE
// fix" — the race window is narrow (Put check → Submit, a few instructions)
// and the default queue capacity (10000) means sends rarely block. Run
// with -count=50 to increase regression detection probability.
func TestConcurrentPutCloseNoPanic(t *testing.T) {
dir := t.TempDir()
cfg := config.Defaults()
ww, err := NewWalWriter(&cfg, dir, 0, 0)
if err != nil {
t.Fatal(err)
}
const goroutines = 100
var wg sync.WaitGroup
var panicCount atomic.Int32
start := make(chan struct{})
for i := 0; i < goroutines; i++ {
wg.Add(1)
go func() {
defer wg.Done()
defer func() {
if r := recover(); r != nil {
panicCount.Add(1)
}
}()
<-start
_ = ww.Put([]byte("k"), []byte("v"))
}()
}
wg.Add(1)
go func() {
defer wg.Done()
<-start
_ = ww.Close()
}()
close(start)
wg.Wait()
if panicCount.Load() > 0 {
t.Fatalf("concurrent Put + Close caused %d panic(s) — C7 regression",
panicCount.Load())
}
}
// Regression guard for C7: same as above but for Delete.
func TestConcurrentDeleteCloseNoPanic(t *testing.T) {
dir := t.TempDir()
cfg := config.Defaults()
ww, err := NewWalWriter(&cfg, dir, 0, 0)
if err != nil {
t.Fatal(err)
}
const goroutines = 100
var wg sync.WaitGroup
var panicCount atomic.Int32
start := make(chan struct{})
for i := 0; i < goroutines; i++ {
wg.Add(1)
go func() {
defer wg.Done()
defer func() {
if r := recover(); r != nil {
panicCount.Add(1)
}
}()
<-start
_ = ww.Delete([]byte("k"))
}()
}
wg.Add(1)
go func() {
defer wg.Done()
<-start
_ = ww.Close()
}()
close(start)
wg.Wait()
if panicCount.Load() > 0 {
t.Fatalf("concurrent Delete + Close caused %d panic(s) — C7 regression",
panicCount.Load())
}
}