Skip to content

Commit 4e60cb7

Browse files
authored
Revert "ethdb/pebble: prevent shutdown-panic (ethereum#27238)"
This reverts commit 98ab75d.
1 parent 7f06703 commit 4e60cb7

File tree

3 files changed

+10
-68
lines changed

3 files changed

+10
-68
lines changed

ethdb/dbtest/testsuite.go

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -376,32 +376,6 @@ func TestDatabaseSuite(t *testing.T, New func() ethdb.KeyValueStore) {
376376
}
377377
}
378378
})
379-
380-
t.Run("OperatonsAfterClose", func(t *testing.T) {
381-
db := New()
382-
db.Put([]byte("key"), []byte("value"))
383-
db.Close()
384-
if _, err := db.Get([]byte("key")); err == nil {
385-
t.Fatalf("expected error on Get after Close")
386-
}
387-
if _, err := db.Has([]byte("key")); err == nil {
388-
t.Fatalf("expected error on Get after Close")
389-
}
390-
if err := db.Put([]byte("key2"), []byte("value2")); err == nil {
391-
t.Fatalf("expected error on Put after Close")
392-
}
393-
if err := db.Delete([]byte("key")); err == nil {
394-
t.Fatalf("expected error on Delete after Close")
395-
}
396-
397-
b := db.NewBatch()
398-
if err := b.Put([]byte("batchkey"), []byte("batchval")); err != nil {
399-
t.Fatalf("expected no error on batch.Put after Close, got %v", err)
400-
}
401-
if err := b.Write(); err == nil {
402-
t.Fatalf("expected error on batch.Write after Close")
403-
}
404-
})
405379
}
406380

407381
// BenchDatabaseSuite runs a suite of benchmarks against a KeyValueStore database

ethdb/memorydb/memorydb.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -244,9 +244,6 @@ func (b *batch) Write() error {
244244
b.db.lock.Lock()
245245
defer b.db.lock.Unlock()
246246

247-
if b.db.db == nil {
248-
return errMemorydbClosed
249-
}
250247
for _, keyvalue := range b.writes {
251248
if keyvalue.delete {
252249
delete(b.db.db, string(keyvalue.key))

ethdb/pebble/pebble.go

Lines changed: 10 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,8 @@ type Database struct {
7070
seekCompGauge metrics.Gauge // Gauge for tracking the number of table compaction caused by read opt
7171
manualMemAllocGauge metrics.Gauge // Gauge for tracking amount of non-managed memory currently allocated
7272

73-
quitLock sync.RWMutex // Mutex protecting the quit channel and the closed flag
73+
quitLock sync.Mutex // Mutex protecting the quit channel access
7474
quitChan chan chan error // Quit channel to stop the metrics collection before closing the database
75-
closed bool // keep track of whether we're Closed
7675

7776
log log.Logger // Contextual logger tracking the database path
7877

@@ -222,29 +221,23 @@ func New(file string, cache int, handles int, namespace string, readonly bool) (
222221
func (d *Database) Close() error {
223222
d.quitLock.Lock()
224223
defer d.quitLock.Unlock()
224+
225225
// Allow double closing, simplifies things
226-
if d.closed {
226+
if d.quitChan == nil {
227227
return nil
228228
}
229-
d.closed = true
230-
if d.quitChan != nil {
231-
errc := make(chan error)
232-
d.quitChan <- errc
233-
if err := <-errc; err != nil {
234-
d.log.Error("Metrics collection failed", "err", err)
235-
}
236-
d.quitChan = nil
229+
errc := make(chan error)
230+
d.quitChan <- errc
231+
if err := <-errc; err != nil {
232+
d.log.Error("Metrics collection failed", "err", err)
237233
}
234+
d.quitChan = nil
235+
238236
return d.db.Close()
239237
}
240238

241239
// Has retrieves if a key is present in the key-value store.
242240
func (d *Database) Has(key []byte) (bool, error) {
243-
d.quitLock.RLock()
244-
defer d.quitLock.RUnlock()
245-
if d.closed {
246-
return false, pebble.ErrClosed
247-
}
248241
_, closer, err := d.db.Get(key)
249242
if err == pebble.ErrNotFound {
250243
return false, nil
@@ -257,11 +250,6 @@ func (d *Database) Has(key []byte) (bool, error) {
257250

258251
// Get retrieves the given key if it's present in the key-value store.
259252
func (d *Database) Get(key []byte) ([]byte, error) {
260-
d.quitLock.RLock()
261-
defer d.quitLock.RUnlock()
262-
if d.closed {
263-
return nil, pebble.ErrClosed
264-
}
265253
dat, closer, err := d.db.Get(key)
266254
if err != nil {
267255
return nil, err
@@ -274,30 +262,19 @@ func (d *Database) Get(key []byte) ([]byte, error) {
274262

275263
// Put inserts the given value into the key-value store.
276264
func (d *Database) Put(key []byte, value []byte) error {
277-
d.quitLock.RLock()
278-
defer d.quitLock.RUnlock()
279-
if d.closed {
280-
return pebble.ErrClosed
281-
}
282265
return d.db.Set(key, value, pebble.NoSync)
283266
}
284267

285268
// Delete removes the key from the key-value store.
286269
func (d *Database) Delete(key []byte) error {
287-
d.quitLock.RLock()
288-
defer d.quitLock.RUnlock()
289-
if d.closed {
290-
return pebble.ErrClosed
291-
}
292270
return d.db.Delete(key, nil)
293271
}
294272

295273
// NewBatch creates a write-only key-value store that buffers changes to its host
296274
// database until a final write is called.
297275
func (d *Database) NewBatch() ethdb.Batch {
298276
return &batch{
299-
b: d.db.NewBatch(),
300-
db: d,
277+
b: d.db.NewBatch(),
301278
}
302279
}
303280

@@ -504,7 +481,6 @@ func (d *Database) meter(refresh time.Duration) {
504481
// when Write is called. A batch cannot be used concurrently.
505482
type batch struct {
506483
b *pebble.Batch
507-
db *Database
508484
size int
509485
}
510486

@@ -529,11 +505,6 @@ func (b *batch) ValueSize() int {
529505

530506
// Write flushes any accumulated data to disk.
531507
func (b *batch) Write() error {
532-
b.db.quitLock.RLock()
533-
defer b.db.quitLock.RUnlock()
534-
if b.db.closed {
535-
return pebble.ErrClosed
536-
}
537508
return b.b.Commit(pebble.NoSync)
538509
}
539510

0 commit comments

Comments
 (0)