search

fiatjaf / eventstore

an abstraction layer over databases for nostr events
The Unlicense
26 stars 15 forks source link

fixes data race that sometimes causes a badger nil panic due to a race on the transaction #11

Closed mleku closed 6 months ago

mleku commented 7 months ago

the transaction is accessed by the iterator closing deferred function as well as by the queries goroutine loop

most of the time this probably doesn't race, in my tests it was hit and miss but adding a mutex to accesses to the txn within the View iterator eliminates the races

this race can cause the QuerySync call in the relay_interface.go to fail to receive all of the transactions due to the race sometimes happening and failing to send a result back over the channel, this should not happen now

mleku commented 7 months ago

this PR also includes a fix that makes the nil pointer test pass, note that a heap of the other tests in the sql and elasticsearch don't match up with expected in the test still

mleku commented 7 months ago

note that there still appears to be one more issue to address, it is sending back nil events now which are causing my relay to panic

going to check for nils before the channels are loaded and add this patch

mleku commented 7 months ago

this should be the complete patch, you still have failing tests in the sql drivers

fiatjaf commented 6 months ago

This is fixed in 830d51e96d46f62f1049ff530f15abef8cf0779a.

Correct usage of badger requires not reusing transactions across goroutines and I hadn't realized that. The mutex couldn't be a workaround, but it's better to do as they say.

mleku commented 6 months ago

This is fixed in 830d51e.

Correct usage of badger requires not reusing transactions across goroutines and I hadn't realized that. The mutex couldn't be a workaround, but it's better to do as they say.

yes in my codebase now i have removed the goroutine inside the transaction completely