Fix support for ReadOnly ThreadSafeReferences

[DominicFrei]

Fix for realm/realm-swift#5475.

What, How & Why?

Problem

This issue is caused by two checks interfering with each other:

realm-core/src/realm/object-store/thread_safe_reference.cpp

Lines 38 to 46 in b64aa00

	class ThreadSafeReference::Payload {
	public:
	virtual ~Payload() = default;
	Payload(Realm& realm)
	: m_transaction(realm.is_in_read_transaction() ? realm.duplicate() : nullptr)
	, m_coordinator(Realm::Internal::get_coordinator(realm).shared_from_this())
	, m_created_in_write_transaction(realm.is_in_transaction())
	{
	}

realm-core/src/realm/object-store/shared_realm.cpp

Lines 490 to 495 in b64aa00

	static void check_can_create_any_transaction(const Realm* realm)
	{
	if (realm->config().immutable()) {
	throw InvalidTransactionException("Can't perform transactions on read-only Realms.");
	}
	}

Within a ThreadSafeReference we duplicate() the realm if it is_in_read_transaction() which it is since the check looks like this:

realm-core/src/realm/object-store/shared_realm.hpp

Lines 313 to 316 in b64aa00

	bool is_in_read_transaction() const
	{
	return m_group != nullptr;
	}

The then executed path of duplicate() which needs to get_version_of_current_transaction() tries to verify_can_create_any_transaction which subsequently crashes because we "Can't perform transactions on read-only Realms.".

Solution

• move ReadLockInfo and get_current_version() to Group so that we can read a version for a (read-only) Group as well
• read_transaction_version() was split into get_current_version() in general and get_version_of_current_or_frozen_transaction() specifically to account for that change
• is_in_any_transaction() was adjusted to match the test in the more specific is_in_write_transaction()

With these preparations we can then make the two resolving changes:

• When duplicating we have to distinguish between read-only and writable Realms:

  realm-core/src/realm/object-store/shared_realm.cpp

  Lines 103 to 115 in fbc94bf

  	std::shared_ptr<Transaction> Realm::duplicate() const
  	{
  	VersionID version;
  	if (m_config.is_immutable()) {
  	version = get_current_version();
  	}
  	else {
  	version = get_version_of_current_transaction();
  	}
  	std::shared_ptr<Group> group = m_coordinator->begin_read(version, is_frozen());
  	std::shared_ptr<Transaction> transaction = std::static_pointer_cast<Transaction>(group);
  	return transaction;
  	}

• With these changes we can then check and duplicate a SharedRealm correctly in

  realm-core/src/realm/object-store/thread_safe_reference.cpp

  Lines 45 to 46 in fbc94bf

  	: m_transaction(realm.is_in_any_transaction() || (realm.config().is_immutable() && realm.has_group())
  	? realm.duplicate()

☑️ ToDos

• 📝 Changelog update
• 🚦 Tests (or not relevant)
• Verify all changes work in SDKs as well - tested for Cocoa on https://github.com/realm/realm-cocoa/tree/df/read-only-thread-safe-references

Notes

I was encouraged to change names when unclear and add comments, etc.
The changes I've made happened while creating this PR and helped me understanding those parts of the core better and eventually led to the solution but are not necessarily part of it. I'm happy to divide them into two PRs if preferred.

Open questions

• realm-core/src/realm/object-store/collection.cpp

  Lines 59 to 67 in fbc94bf

  	bool Collection::is_valid() const
  	{
  	if (!m_realm)
  	return false;
  	m_realm->verify_is_on_thread();
  	if (!m_realm->is_in_any_transaction())
  	return false;
  	return m_coll_base->is_attached();
  	}

  
  Why can a List, Set or Dictionary not be valid on a read-only realm?
• When writing the tests I noticed that there aren't any for writable realms for those Collections. Is this connected to the is_valid() function?

[jsflax]

The title of this PR should be in the imperative: Fix support for ReadOnly ThreadSafeReferences (or something similar).

[tgoyne]

It is difficult to find the actual change here in the giant mess of unrelated pointless renamings, but I think it's incorrect. The entire concept of a readlock or version does not apply to a read-only Realm, and it doesn't make any sense to add them to Group. import_copy_of() on Group just doesn't need to check the version at all because there's only ever one version of a non-Transaction Group.

[finnschiermer]

also, as I said earlier, we can't merge this. I want to unify handling of read-only in DB instead of needing special casing with Group all over. @jsflax, for which I created #4528.

[DominicFrei]

Issue was continued and fixed by @finnschiermer in #4688.