diff --git a/database/interface.go b/database/interface.go index 9c6b4a9..d0a6552 100644 --- a/database/interface.go +++ b/database/interface.go @@ -188,6 +188,16 @@ func (i *Interface) getRecord(dbName string, dbKey string, mustBeWriteable bool) return nil, db, ErrPermissionDenied } + r.Lock() + ttl := r.Meta().GetRelativeExpiry() + r.Unlock() + i.updateCache( + r, + false, // writing + false, // remove + ttl, // expiry + ) + return r, db, nil } @@ -243,14 +253,19 @@ func (i *Interface) Put(r record.Record) (err error) { } r.Lock() - defer r.Unlock() i.options.Apply(r) + remove := r.Meta().IsDeleted() + ttl := r.Meta().GetRelativeExpiry() + r.Unlock() - written := i.updateCache(r, true) + // The record may not be locked when updating the cache. + written := i.updateCache(r, true, remove, ttl) if written { return nil } + r.Lock() + defer r.Unlock() return db.Put(r) } @@ -271,18 +286,22 @@ func (i *Interface) PutNew(r record.Record) (err error) { } r.Lock() - defer r.Unlock() - if r.Meta() != nil { r.Meta().Reset() } i.options.Apply(r) + remove := r.Meta().IsDeleted() + ttl := r.Meta().GetRelativeExpiry() + r.Unlock() - written := i.updateCache(r, true) + // The record may not be locked when updating the cache. + written := i.updateCache(r, true, remove, ttl) if written { return nil } + r.Lock() + defer r.Unlock() return db.Put(r) } diff --git a/database/interface_cache.go b/database/interface_cache.go index 3e349e5..ec2eb7c 100644 --- a/database/interface_cache.go +++ b/database/interface_cache.go @@ -159,14 +159,19 @@ func (i *Interface) checkCache(key string) record.Record { return nil } -func (i *Interface) updateCache(r record.Record, write bool) (written bool) { +// updateCache updates an entry in the interface cache. The given record may +// not be locked, as updating the cache might write an (unrelated) evicted +// record to the database in the process. If this happens while the +// DelayedCacheWriter flushes the write cache with the same record present, +// this will deadlock. +func (i *Interface) updateCache(r record.Record, write bool, remove bool, ttl int64) (written bool) { // Check if cache is in use. if i.cache == nil { return false } // Check if record should be deleted - if r.Meta().IsDeleted() { + if remove { // Remove entry from cache. i.cache.Remove(r.Key()) // Let write through to database storage. @@ -174,7 +179,6 @@ func (i *Interface) updateCache(r record.Record, write bool) (written bool) { } // Update cache with record. - ttl := r.Meta().GetRelativeExpiry() if ttl >= 0 { _ = i.cache.SetWithExpire( r.Key(), diff --git a/database/record/base.go b/database/record/base.go index 25a07dd..c1108f0 100644 --- a/database/record/base.go +++ b/database/record/base.go @@ -6,6 +6,7 @@ import ( "github.com/safing/portbase/container" "github.com/safing/portbase/database/accessor" "github.com/safing/portbase/formats/dsd" + "github.com/safing/portbase/log" ) // TODO(ppacher): @@ -31,37 +32,46 @@ type Base struct { meta *Meta } +// SetKey sets the key on the database record. The key may only be set once and +// future calls to SetKey will be ignored. If you want to copy/move the record +// to another database key, you will need to create a copy and assign a new key. +// A key must be set before the record is used in any database operation. +func (b *Base) SetKey(key string) { + if !b.KeyIsSet() { + b.dbName, b.dbKey = ParseKey(key) + } else { + log.Errorf("database: key is already set: tried to replace %q with %q", b.Key(), key) + } +} + // Key returns the key of the database record. +// As the key must be set before any usage and can only be set once, this +// function may be used without locking the record. func (b *Base) Key() string { return b.dbName + ":" + b.dbKey } // KeyIsSet returns true if the database key is set. +// As the key must be set before any usage and can only be set once, this +// function may be used without locking the record. func (b *Base) KeyIsSet() bool { - return len(b.dbName) > 0 && len(b.dbKey) > 0 + return b.dbName != "" } // DatabaseName returns the name of the database. +// As the key must be set before any usage and can only be set once, this +// function may be used without locking the record. func (b *Base) DatabaseName() string { return b.dbName } // DatabaseKey returns the database key of the database record. +// As the key must be set before any usage and can only be set once, this +// function may be used without locking the record. func (b *Base) DatabaseKey() string { return b.dbKey } -// SetKey sets the key on the database record, it should only be called after loading the record. Use MoveTo to save the record with another key. -func (b *Base) SetKey(key string) { - b.dbName, b.dbKey = ParseKey(key) -} - -// MoveTo sets a new key for the record and resets all metadata, except for the secret and crownjewel status. -func (b *Base) MoveTo(key string) { - b.SetKey(key) - b.meta.Reset() -} - // Meta returns the metadata object for this record. func (b *Base) Meta() *Meta { return b.meta @@ -75,7 +85,7 @@ func (b *Base) CreateMeta() { // UpdateMeta creates the metadata if it does not exist and updates it. func (b *Base) UpdateMeta() { if b.meta == nil { - b.meta = &Meta{} + b.CreateMeta() } b.meta.Update() } diff --git a/database/record/record.go b/database/record/record.go index 850e65c..65808f2 100644 --- a/database/record/record.go +++ b/database/record/record.go @@ -6,13 +6,12 @@ import ( // Record provides an interface for uniformally handling database records. type Record interface { - Key() string // test:config + SetKey(key string) // test:config + Key() string // test:config KeyIsSet() bool DatabaseName() string // test DatabaseKey() string // config - SetKey(key string) // test:config - MoveTo(key string) // test:config Meta() *Meta SetMeta(meta *Meta) CreateMeta()