Update transaction stage earlier in commit_and_continue_as_read

[jedelbo]

This will ensure the the stage is correct even if subsequent
procedure calls will trigger an exception.

☑️ ToDos

• 📝 Changelog update
• 🚦 Tests (or not relevant)
• C-API, if public C++ API changed.

[ironage]

Hmm I don't quite get what you have in mind here. But I also don't see how it can hurt either. I'll leave a tentative approval, but it might be best to get another set of eyes on this one.

[tgoyne]

AFAICT, everything between db->grab_read_lock(new_read_lock, version_id); and remap_and_update_refs(m_read_lock.m_top_ref, m_read_lock.m_file_size, false); should not be able to fail. Many of the operations in between aren't noexcept, but the only exceptions they throw are things like failing to lock a mutex. There's maybe an argument that they should be noexcept as we aren't prepared to handle those failing and would prefer to terminate.

That means that the main effect of this is that if remap_and_update_refs() fails (which it quite plausible can if the file has grown), the Transaction is left in the Read stage rather than the Write stage. This is still probably not really correct as continuing to use the Transaction past this point is very unlikely to work, but it is better than leaving it in write mode. Crashes in rollback would certainly make sense if the transaction being rolled back had actually been committed but then the transaction was left in the write stage because remap_and_update_refs() threw.

[finnschiermer]

I changed this so that an exception in commit_and_continue_as_read will cause the Transaction object to reject both reading and writing afterwards.