ssbc / ssb-db2

A new database for secure-scuttlebutt
47 stars 8 forks source link

Crash in ssb-db2 private when resuming compaction #380

Closed staltz closed 2 years ago

staltz commented 2 years ago

Reproduction

Reproduction steps:

  1. Delete some feeds
  2. Start compaction
  3. In the middle compaction, kill/crash the app
  4. Reopen the app
  5. Compaction resumes but the following crash happens

Screenshot from 2022-08-04 11-37-34

This is the line: https://github.com/ssbc/ssb-db2/blob/44c4b0438f00c3dffe10079d70449fb1c7cb480b/indexes/private.js#L151

And it was called from https://github.com/ssbc/ssb-db2/blob/44c4b0438f00c3dffe10079d70449fb1c7cb480b/indexes/private.js#L179

Which means it's reading from the canDecrypt index


Analysis

This means that the ciphertext is not a string. But the record is non-deleted because there is a check for non-null records.

So what I think this means is that the msg.value.content is an object, thus non-encrypted record. More guessing tells me that when the app re-opened, we allowed db2 queries to happen, and those might be using outdated offsets (because reindexing has not yet kicked in). And no db2 queries should happen at this point.

I think that the culprit is

https://github.com/ssbc/ssb-db2/blob/44c4b0438f00c3dffe10079d70449fb1c7cb480b/db.js#L104

because in this case the default value should be true, but because it's false then the queries are allowed to proceed:

https://github.com/ssbc/ssb-db2/blob/44c4b0438f00c3dffe10079d70449fb1c7cb480b/db.js#L696-L697

But it seems like we can just use the synchronous log.compactionProgress.value.done because it is synchronously defined by AAOL on startup:

https://github.com/ssbc/async-append-only-log/blob/2ad95069e94778b6d3cd62909b96c5da0b273fc2/index.js#L71-L75

staltz commented 2 years ago

Assume fixed by #381