mbdavid / LiteDB

LiteDB - A .NET NoSQL Document Store in a single data file
http://www.litedb.org
MIT License
8.35k stars 1.22k forks source link

Fix "ENSURE: empty page must be defined as empty type" #2504

Open oleksii-datsiuk opened 2 weeks ago

oleksii-datsiuk commented 2 weeks ago

This PR fixes "ENSURE: empty page must be defined as empty type" error which is described in this issue: https://github.com/mbdavid/LiteDB/issues/2503

oleksii-datsiuk commented 2 weeks ago

In addition to fixing the problem described in the issue, this PR has 2 more improvements:

  1. In Snapshot.NewPage() there is comment: // there is need for _header.Savepoint() because changes here will incremental and will be persist later // if any problem occurs here, rollback will catch this changes" However there is no savepoint in this function (and according to the history never was). So I added savepoint here. But not exactly in the place where comment is written, but in the "else" block inside this function, where savepoint seems to be important.

  2. I made WalIndexService.CurrentReadVersion synchronized. Its backing field (_currentReadVersion) is accessed (read and modified) in the same class in Clear() and ConfirmTransaction() functions. These functions are atomic by using _indexLock. Especially ConfirmTransaction is interesting, because it first increases _currentReadVersion and then adds some data based on this increased value to the WAL index. So it seems unsafe to be able to access WalIndexService.CurrentReadVersion property while ConfirmTransaction() is still being executed.