Commit a5dbb92b authored by Kirill Smelkov's avatar Kirill Smelkov

go/zodb: Require drivers to close watchq on Close

If we don't require drivers to stop sending to watchq after Close, there
could be many deadlock scenarios, for example:

- client called drv.Close(); no longer listens to watchq; driver is
  stuck sending to watchq, or

- client called drv.Close(), which itself waits for tasks spawned inside
  driver to complete, which are stuck sending to watchq because client
  no longer does <-watchq.

The change is similar in spirit to safety guaranty provided by high-level
Watcher where after DelWatch call it is guaranteed that there will be no
more sends to subscribed watchq (see c41c2907 "go/zodb: High-level
watching - initial draft") for details.

All drivers don't provide requested guarantee yet.
We'll be fixing them one-by-one in followup commits.
parent 20d48ded
...@@ -31,6 +31,7 @@ import ( ...@@ -31,6 +31,7 @@ import (
"reflect" "reflect"
"sync" "sync"
"testing" "testing"
"time"
"lab.nexedi.com/kirr/go123/xerr" "lab.nexedi.com/kirr/go123/xerr"
"lab.nexedi.com/kirr/neo/go/zodb" "lab.nexedi.com/kirr/neo/go/zodb"
...@@ -433,6 +434,11 @@ func DrvTestWatch(t *testing.T, zurl string, zdrvOpen zodb.DriverOpener) { ...@@ -433,6 +434,11 @@ func DrvTestWatch(t *testing.T, zurl string, zdrvOpen zodb.DriverOpener) {
} }
} }
// commit something more and wait a bit to raise chances the driver enqueues to watchq<- .
_ = xcommit(at, ZRawObject{0, b("at the end")})
time.Sleep(1*time.Second)
// the driver must handle Close and cancel that watchq<-
err = zdrv.Close(); X(err) err = zdrv.Close(); X(err)
e, ok := <-watchq e, ok := <-watchq
......
// Copyright (C) 2017-2019 Nexedi SA and Contributors. // Copyright (C) 2017-2021 Nexedi SA and Contributors.
// Kirill Smelkov <kirr@nexedi.com> // Kirill Smelkov <kirr@nexedi.com>
// //
// This program is free software: you can Use, Study, Modify and Redistribute // This program is free software: you can Use, Study, Modify and Redistribute
...@@ -46,12 +46,30 @@ type DriverOptions struct { ...@@ -46,12 +46,30 @@ type DriverOptions struct {
// //
// Watchq can be nil to ignore such events. However if Watchq != nil, the events // Watchq can be nil to ignore such events. However if Watchq != nil, the events
// have to be consumed or else the storage driver will misbehave - e.g. // have to be consumed or else the storage driver will misbehave - e.g.
// it can get out of sync with the on-disk database file. // it can get out of sync with the on-disk database file, or deadlock
// on any user-called operation.
// //
// The storage driver closes !nil Watchq when the driver is closed. // The storage driver closes !nil Watchq when the driver is closed.
// //
// The storage driver will send only and all events in (at₀, +∞] range, // The storage driver will send only and all events in (at₀, +∞] range,
// where at₀ is at returned by driver open. // where at₀ is at returned by driver open.
//
// The storage driver will stop sending events after call to Close.
// In particular the following example is valid and safe from deadlock:
//
// watchq := make(chan zodb.Event)
// stor, at0, err := zodb.OpenDriver(..., &DriverOptions{Watchq: watchq})
// defer stor.Close()
//
// for {
// select {
// case <-ctx.Done():
// return ctx.Err()
//
// case <-watchq:
// ...
// }
// }
Watchq chan<- Event Watchq chan<- Event
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment