From 8a30bc943f45033f630db306d6211c65b611c190 Mon Sep 17 00:00:00 2001 From: William Choe Date: Thu, 26 Mar 2026 14:55:49 -0400 Subject: [PATCH] base: add Exit function to replace direct os.Exit calls Add a package-level Exit function in internal/base that wraps os.Exit. Embedders (e.g. CockroachDB) can override base.Exit to route invariant violation exits through their own fatal logging infrastructure, giving them visibility into why the process died (log lines, crash reports, marker files). Previously, 20 call sites in production code called os.Exit(1) directly, which bypassed any logging or crash reporting the embedder had set up. The process would simply vanish with exit code 1 and no indication of why. Informs cockroachdb/pebble#2061 --- internal/base/exit.go | 15 +++++++++++++++ internal/base/logger.go | 3 +-- internal/cache/block_map.go | 3 ++- internal/cache/cache.go | 2 +- internal/cache/clockpro.go | 10 +++++----- internal/cache/entry.go | 3 ++- internal/cache/value.go | 3 ++- mem_table.go | 2 +- objstorage/objstorageprovider/vfs_readable.go | 5 +++-- open.go | 2 +- options.go | 11 +++++++++++ sstable/colblk/keyspan.go | 2 +- sstable/reader_iter.go | 8 ++++---- sstable/rowblk/rowblk_fragment_iter.go | 2 +- 14 files changed, 50 insertions(+), 21 deletions(-) create mode 100644 internal/base/exit.go diff --git a/internal/base/exit.go b/internal/base/exit.go new file mode 100644 index 00000000000..761a4f48327 --- /dev/null +++ b/internal/base/exit.go @@ -0,0 +1,15 @@ +// Copyright 2026 The LevelDB-Go and Pebble Authors. All rights reserved. Use +// of this source code is governed by a BSD-style license that can be found in +// the LICENSE file. + +package base + +import "os" + +// Exit is called when Pebble needs to terminate the process due to an +// invariant violation. By default it calls os.Exit, but embedders +// (e.g. CockroachDB) can override it to route through their own fatal +// logging infrastructure for better crash visibility. +var Exit = func(code int) { + os.Exit(code) +} diff --git a/internal/base/logger.go b/internal/base/logger.go index 3d475648661..1f77148c00a 100644 --- a/internal/base/logger.go +++ b/internal/base/logger.go @@ -9,7 +9,6 @@ import ( "context" "fmt" "log" - "os" "sync" "github.com/cockroachdb/pebble/internal/invariants" @@ -41,7 +40,7 @@ func (defaultLogger) Errorf(format string, args ...interface{}) { // Fatalf implements the Logger.Fatalf interface. func (defaultLogger) Fatalf(format string, args ...interface{}) { _ = log.Output(2, fmt.Sprintf(format, args...)) - os.Exit(1) + Exit(1) } // InMemLogger implements Logger using an in-memory buffer (used for testing). diff --git a/internal/cache/block_map.go b/internal/cache/block_map.go index 7fe701f9a79..d644c953c34 100644 --- a/internal/cache/block_map.go +++ b/internal/cache/block_map.go @@ -9,6 +9,7 @@ import ( "os" "unsafe" + "github.com/cockroachdb/pebble/internal/base" "github.com/cockroachdb/pebble/internal/invariants" "github.com/cockroachdb/pebble/internal/manual" "github.com/cockroachdb/swiss" @@ -58,7 +59,7 @@ func newBlockMap(initialCapacity int) *blockMap { m := obj.(*blockMap) if !m.closed { fmt.Fprintf(os.Stderr, "%p: block-map not closed\n", m) - os.Exit(1) + base.Exit(1) } }) return m diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 047e7fb57d4..8be48c9c368 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -141,7 +141,7 @@ func NewWithShards(size int64, shards int) *Cache { fmt.Fprintf(os.Stderr, "%s\n", strings.Join(c.tr.msgs, "\n")) } c.tr.Unlock() - os.Exit(1) + base.Exit(1) } }) return c diff --git a/internal/cache/clockpro.go b/internal/cache/clockpro.go index ca1e9e069d0..99247ac0f9d 100644 --- a/internal/cache/clockpro.go +++ b/internal/cache/clockpro.go @@ -500,17 +500,17 @@ func (c *shard) metaCheck(e *entry) { if _, ok := c.entries[e]; ok { fmt.Fprintf(os.Stderr, "%p: %s unexpectedly found in entries map\n%s", e, e.key, debug.Stack()) - os.Exit(1) + base.Exit(1) } if c.blocks.findByValue(e) { fmt.Fprintf(os.Stderr, "%p: %s unexpectedly found in blocks map\n%#v\n%s", e, e.key, &c.blocks, debug.Stack()) - os.Exit(1) + base.Exit(1) } if c.files.findByValue(e) { fmt.Fprintf(os.Stderr, "%p: %s unexpectedly found in files map\n%#v\n%s", e, e.key, &c.files, debug.Stack()) - os.Exit(1) + base.Exit(1) } // NB: c.hand{Hot,Cold,Test} are pointers into a single linked list. We // only have to traverse one of them to check all of them. @@ -532,7 +532,7 @@ func (c *shard) metaCheck(e *entry) { if e == t { fmt.Fprintf(os.Stderr, "%p: %s unexpectedly found in blocks list\n%s", e, e.key, debug.Stack()) - os.Exit(1) + base.Exit(1) } if t == c.handHot { break @@ -546,7 +546,7 @@ func (c *shard) metaCheck(e *entry) { c.countHot, c.sizeHot, c.countCold, c.sizeCold, c.countTest, c.sizeTest, countHot, sizeHot, countCold, sizeCold, countTest, sizeTest, debug.Stack()) - os.Exit(1) + base.Exit(1) } } } diff --git a/internal/cache/entry.go b/internal/cache/entry.go index ba52b3317b5..b62088e1459 100644 --- a/internal/cache/entry.go +++ b/internal/cache/entry.go @@ -12,6 +12,7 @@ import ( "sync/atomic" "unsafe" + "github.com/cockroachdb/pebble/internal/base" "github.com/cockroachdb/pebble/internal/buildtags" "github.com/cockroachdb/pebble/internal/invariants" "github.com/cockroachdb/pebble/internal/manual" @@ -186,7 +187,7 @@ func entryAllocNew() *entry { e := obj.(*entry) if *e != (entry{}) { fmt.Fprintf(os.Stderr, "%p: entry was not freed", e) - os.Exit(1) + base.Exit(1) } }) return e diff --git a/internal/cache/value.go b/internal/cache/value.go index 03a3bc7bb11..1e8de7a7f43 100644 --- a/internal/cache/value.go +++ b/internal/cache/value.go @@ -11,6 +11,7 @@ import ( "unsafe" "github.com/cockroachdb/errors" + "github.com/cockroachdb/pebble/internal/base" "github.com/cockroachdb/pebble/internal/buildtags" "github.com/cockroachdb/pebble/internal/invariants" "github.com/cockroachdb/pebble/internal/manual" @@ -70,7 +71,7 @@ func Alloc(n int) *Value { if v.buf != nil { fmt.Fprintf(os.Stderr, "%p: cache value was not freed: refs=%d\n%s", v, v.refs(), v.ref.traces()) - os.Exit(1) + base.Exit(1) } }) return v diff --git a/mem_table.go b/mem_table.go index 984cd4fdf5a..ef5999057ab 100644 --- a/mem_table.go +++ b/mem_table.go @@ -112,7 +112,7 @@ func checkMemTable(obj interface{}) { m := obj.(*memTable) if m.arenaBuf.Data() != nil { fmt.Fprintf(os.Stderr, "%v: memTable buffer was not freed\n", m.arenaBuf) - os.Exit(1) + base.Exit(1) } } diff --git a/objstorage/objstorageprovider/vfs_readable.go b/objstorage/objstorageprovider/vfs_readable.go index 0e9cd905cc7..413b81fa304 100644 --- a/objstorage/objstorageprovider/vfs_readable.go +++ b/objstorage/objstorageprovider/vfs_readable.go @@ -11,6 +11,7 @@ import ( "runtime/debug" "sync" + "github.com/cockroachdb/pebble/internal/base" "github.com/cockroachdb/pebble/internal/invariants" "github.com/cockroachdb/pebble/objstorage" "github.com/cockroachdb/pebble/vfs" @@ -55,7 +56,7 @@ func newFileReadable( invariants.SetFinalizer(r, func(obj interface{}) { if obj.(*fileReadable).file != nil { fmt.Fprintf(os.Stderr, "Readable %s was not closed\n%s", filename, stack) - os.Exit(1) + base.Exit(1) } }) } @@ -112,7 +113,7 @@ var readHandlePool = sync.Pool{ invariants.SetFinalizer(i, func(obj interface{}) { if obj.(*vfsReadHandle).r != nil { fmt.Fprintf(os.Stderr, "ReadHandle was not closed") - os.Exit(1) + base.Exit(1) } }) } diff --git a/open.go b/open.go index 3edf11ed497..82396a37410 100644 --- a/open.go +++ b/open.go @@ -521,7 +521,7 @@ func Open(dirname string, opts *Options) (db *DB, err error) { v := obj.(*atomic.Value) if err := v.Load(); err == nil { fmt.Fprintf(os.Stderr, "%s: unreferenced DB not closed\n", dPtr) - os.Exit(1) + base.Exit(1) } }) diff --git a/options.go b/options.go index 5733690f741..7e83318269d 100644 --- a/options.go +++ b/options.go @@ -8,6 +8,7 @@ import ( "bytes" "fmt" "io" + "os" "regexp" "runtime" "slices" @@ -943,6 +944,12 @@ type Options struct { // LoggerAndTracer is used for writing log messages and traces. LoggerAndTracer LoggerAndTracer + // ExitFunc is called when Pebble needs to terminate the process due to + // an invariant violation. If nil, os.Exit is used. Embedders (e.g. + // CockroachDB) can set this to route exits through their own fatal + // logging infrastructure for better crash visibility. + ExitFunc func(code int) + // MaxManifestFileSize is the maximum size the MANIFEST file is allowed to // become. When the MANIFEST exceeds this size it is rolled over and a new // MANIFEST is created. @@ -1675,6 +1682,10 @@ func (o *Options) EnsureDefaults() { if o.Logger == nil { o.Logger = DefaultLogger } + if o.ExitFunc == nil { + o.ExitFunc = os.Exit + } + base.Exit = o.ExitFunc if o.EventListener == nil { o.EventListener = &EventListener{} } diff --git a/sstable/colblk/keyspan.go b/sstable/colblk/keyspan.go index 53d3aa3b982..18fbf054ad3 100644 --- a/sstable/colblk/keyspan.go +++ b/sstable/colblk/keyspan.go @@ -342,7 +342,7 @@ var keyspanIterPool = sync.Pool{ invariants.SetFinalizer(i, func(obj interface{}) { if i := obj.(*KeyspanIter); i.handle.Valid() { fmt.Fprintf(os.Stderr, "KeyspanIter.handle is not nil: %#v\n", i.handle) - os.Exit(1) + base.Exit(1) } }) } diff --git a/sstable/reader_iter.go b/sstable/reader_iter.go index 488dd1f36ca..5e8d3d07066 100644 --- a/sstable/reader_iter.go +++ b/sstable/reader_iter.go @@ -204,11 +204,11 @@ func checkSingleLevelIterator[I any, PI indexBlockIterator[I], D any, PD dataBlo i := obj.(*singleLevelIterator[I, PI, D, PD]) if h := PD(&i.data).Handle(); h.Valid() { fmt.Fprintf(os.Stderr, "singleLevelIterator.data.handle is not nil: %#v\n", h) - os.Exit(1) + base.Exit(1) } if h := PI(&i.index).Handle(); h.Valid() { fmt.Fprintf(os.Stderr, "singleLevelIterator.index.handle is not nil: %#v\n", h) - os.Exit(1) + base.Exit(1) } } @@ -218,10 +218,10 @@ func checkTwoLevelIterator[I any, PI indexBlockIterator[I], D any, PD dataBlockI i := obj.(*twoLevelIterator[I, PI, D, PD]) if h := PD(&i.secondLevel.data).Handle(); h.Valid() { fmt.Fprintf(os.Stderr, "singleLevelIterator.data.handle is not nil: %#v\n", h) - os.Exit(1) + base.Exit(1) } if h := PI(&i.secondLevel.index).Handle(); h.Valid() { fmt.Fprintf(os.Stderr, "singleLevelIterator.index.handle is not nil: %#v\n", h) - os.Exit(1) + base.Exit(1) } } diff --git a/sstable/rowblk/rowblk_fragment_iter.go b/sstable/rowblk/rowblk_fragment_iter.go index eb3984ed7fb..2f9b81fc08a 100644 --- a/sstable/rowblk/rowblk_fragment_iter.go +++ b/sstable/rowblk/rowblk_fragment_iter.go @@ -422,6 +422,6 @@ func checkFragmentBlockIterator(obj interface{}) { i := obj.(*fragmentIter) if h := i.blockIter.Handle(); h.Valid() { fmt.Fprintf(os.Stderr, "fragmentBlockIter.blockIter.handle is not nil: %#v\n", h) - os.Exit(1) + base.Exit(1) } }