Progress notifiers are called after download is complete when opening a synced realm

[papafe]

One .NET user reported a native crash just after opening a synced realm (ticket). We figured out this is happening because a managed handle for progress notifiers is being called after being freed. The handle is freed after the download completion is called and the synced realm is opened. We expect that the progress notifiers would be deregistered once the AsyncOpenTask is resolved.
For context, we are calling AsyncOpenTask::register_download_progress_notifier with is_streaming: false, so we expect that once the task is resolved, all the data download should have been completed.

[nirinchev]

Looking at this, I can see what the issue is. When wait_for_download_completion fires, we exchange m_session for a nullptr:

realm-core/src/realm/object-store/sync/async_open_task.cpp

Lines 42 to 45 in 7d65a82

	session->wait_for_download_completion([callback, self, this](std::error_code ec) {
	auto session = m_session.exchange(nullptr);
	if (!session)
	return; // Swallow all events if the task as been canceled.

Which then means that we can't unregister the progress notifier from the SDK because m_session is now null:

realm-core/src/realm/object-store/sync/async_open_task.cpp

Lines 85 to 89 in 7d65a82

	void AsyncOpenTask::unregister_download_progress_notifier(uint64_t token)
	{
	if (auto session = m_session.load())
	session->unregister_progress_notifier(token);
	}

We should find a way to ensure that all progress notifiers registered on the AsyncOpenTask are unregistered by the time it's resolved. One option is to keep a hashset of tokens and just call unregister_download_progress_notifier with all of them before we cancel or resolve the task.

@tgoyne let me know if that makes sense to you or if there's a better/less naive way to do it.

[tgoyne]

It should just be a vector of tokens and not some more complicated data structure, but otherwise makes sense to me. In the other SDKs we're probably currently soft-leaking progress notifiers in a way that just doesn't crash, but progress notifiers added to an async open shouldn't be firing for any progress after that open completes.