Related to #30
These both make modifications to the database, each in its own separate transaction, but they can be called independently and the sequence number is often used by other concurrent processes giving rise to a huge opportunity for corrupting the database in a concurrent usage scenario:
|
func (idx *index) Set(ctx context.Context, addr indexes.Addr, v interface{}) error { |
|
var ( |
|
raw []byte |
|
err error |
|
) |
|
|
|
if m, ok := v.(encoding.BinaryMarshaler); ok { |
|
raw, err = m.MarshalBinary() |
|
if err != nil { |
|
return fmt.Errorf("error marshaling value using custom marshaler:%w", err) |
|
} |
|
} else { |
|
raw, err = json.Marshal(v) |
|
if err != nil { |
|
return fmt.Errorf("error marshaling value using json marshaler:%w", err) |
|
} |
|
} |
|
|
|
err = idx.db.Set([]byte(addr), raw) |
|
if err != nil { |
|
return fmt.Errorf("error in store:%w", err) |
|
} |
|
|
|
idx.l.Lock() |
|
defer idx.l.Unlock() |
|
|
|
obv, ok := idx.obvs[addr] |
|
if ok { |
|
err = obv.Set(v) |
|
if err != nil { |
|
return fmt.Errorf("error setting value in observable:%w", err) |
|
} |
|
} |
|
|
|
return nil |
|
} |
|
func (idx *index) SetSeq(seq int64) error { |
|
var ( |
|
raw = make([]byte, 8) |
|
err error |
|
addr indexes.Addr = "__current_observable" |
|
) |
|
|
|
binary.BigEndian.PutUint64(raw, uint64(seq)) |
|
|
|
err = idx.db.Set([]byte(addr), raw) |
|
if err != nil { |
|
return fmt.Errorf("error during mkv update (%T): %w", seq, err) |
|
} |
|
|
|
idx.l.Lock() |
|
defer idx.l.Unlock() |
|
|
|
idx.curSeq = seq |
|
|
|
return nil |
|
} |
I seem to recall this Set() followed by SetSeq() pattern used in go-ssb as well. We should probably have a way to do this in a single call which either uses a mutex or opens a transaction, writes both, and commits it. Right now anyone using Set() within a SinkIndex callback is going to be subject to this problem. And it readily hands the user a footgun by not even providing a mechanism to do this properly and forcing them to use this pattern externally as well.
Related to #30
These both make modifications to the database, each in its own separate transaction, but they can be called independently and the sequence number is often used by other concurrent processes giving rise to a huge opportunity for corrupting the database in a concurrent usage scenario:
margaret/indexes/mkv/index.go
Lines 96 to 131 in 22ed48a
margaret/indexes/mkv/index.go
Lines 153 to 173 in 22ed48a
I seem to recall this
Set()followed bySetSeq()pattern used ingo-ssbas well. We should probably have a way to do this in a single call which either uses a mutex or opens a transaction, writes both, and commits it. Right now anyone usingSet()within aSinkIndexcallback is going to be subject to this problem. And it readily hands the user a footgun by not even providing a mechanism to do this properly and forcing them to use this pattern externally as well.