Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid initial version pinning #1519

Merged
merged 38 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
d80bae5
Avoid initial version pinning
clementetb Sep 13, 2023
5ee6feb
Add changelog entry
clementetb Sep 13, 2023
369b4d8
Let the notifier handle GC snapshots
clementetb Sep 14, 2023
4cfc9f7
Revert "Let the notifier handle GC snapshots"
clementetb Sep 15, 2023
aca8768
Revert "Avoid initial version pinning"
clementetb Sep 15, 2023
15fe501
Split version tracker tracking functions
clementetb Sep 15, 2023
947d0ed
Remove on initial
clementetb Sep 15, 2023
6589248
Trigger clean up of GC tracked references
clementetb Sep 15, 2023
f77011b
Clean up
clementetb Sep 15, 2023
1bd1f6f
Merge branch 'releases' into ct/fix-initialversion-pinned
clementetb Sep 15, 2023
ef03369
Amend race condition
clementetb Sep 15, 2023
58b8b3f
move event
clementetb Sep 15, 2023
ab31164
Crash fix
clementetb Sep 16, 2023
664b8f8
linting
clementetb Sep 16, 2023
fffe381
Fix race condition
clementetb Sep 16, 2023
4b8be5d
linting
clementetb Sep 16, 2023
cf7fbdd
Remove prints
clementetb Sep 17, 2023
7794a45
do not notify on init
clementetb Sep 17, 2023
8c0082d
Fix race condition with lazy property
clementetb Sep 17, 2023
907bf96
Update versiontracker references atomically
clementetb Sep 18, 2023
ac32b82
track all references references
clementetb Sep 18, 2023
6c30a6a
Revert changes
clementetb Sep 18, 2023
aed35f4
Clean up
clementetb Sep 18, 2023
4a192e9
point to possible improvements
clementetb Sep 25, 2023
4e9c025
Use exisiting notification channel
clementetb Sep 27, 2023
38f7da9
Revert atomizing isInitialized
clementetb Sep 27, 2023
d5bd1f7
Remove main from tracking tests
clementetb Sep 27, 2023
c52a6f5
linting
clementetb Sep 27, 2023
5dfa145
Cleanup
clementetb Sep 27, 2023
5d67d1c
PR change requests
clementetb Sep 28, 2023
cecd2e1
Move initial realm instance lifecycle to notifier realm
clementetb Oct 16, 2023
2f29652
linting
clementetb Oct 16, 2023
963e505
Revert "linting"
clementetb Oct 23, 2023
da38e82
Revert "Move initial realm instance lifecycle to notifier realm"
clementetb Oct 23, 2023
4e8c51f
Update test cases
clementetb Oct 25, 2023
514a514
Merge branch 'releases' into ct/fix-initialversion-pinned
clementetb Oct 27, 2023
9305124
Update packages/test-base/src/commonTest/kotlin/io/realm/kotlin/test/…
clementetb Oct 30, 2023
edc44a4
PR change requests
clementetb Oct 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,30 @@
## 1.11.2 (YYYY-MM-DD)

### Enhancements
* None.

### Fixed
* `Realm.getNumberOfActiveVersions` now returns the actual number of active versions. (Core issue [#6960](https://github.com/realm/realm-core/pull/6960))
* Removed pin on the initial realm version after opening a Realm. (Issue [#1519](https://github.com/realm/realm-kotlin/pull/1519))
clementetb marked this conversation as resolved.
Show resolved Hide resolved

### Compatibility
* File format: Generates Realms with file format v23.
* Realm Studio 13.0.0 or above is required to open Realms created by this version.
* This release is compatible with the following Kotlin releases:
* Kotlin 1.8.0 and above. The K2 compiler is not supported yet.
* Ktor 2.1.2 and above.
* Coroutines 1.7.0 and above.
* AtomicFu 0.18.3 and above.
* The new memory model only. See https://github.com/realm/realm-kotlin#kotlin-memory-model-and-coroutine-compatibility
* Minimum Kbson 0.3.0.
* Minimum Gradle version: 6.8.3.
* Minimum Android Gradle Plugin version: 4.1.3.
* Minimum Android SDK: 16.

### Internal
* Updated to Realm Core 13.20.0, commit a3717e492ec606e42995169cf706a82fc1cd0698.


## 1.11.1 (2023-09-07)

### Enhancements
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ internal abstract class LiveRealm(
internal fun gcTrackedSnapshot(): FrozenRealmReference {
return snapshotLock.withLock {
_snapshot.value.also { snapshot ->
if (_closeSnapshotWhenAdvancing) {
if (_closeSnapshotWhenAdvancing && !snapshot.isClosed()) {
log.trace("${this@LiveRealm} ENABLE-TRACKING ${snapshot.version()}")
_closeSnapshotWhenAdvancing = false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package io.realm.kotlin.internal
import io.realm.kotlin.Configuration
import io.realm.kotlin.MutableRealm
import io.realm.kotlin.Realm
import io.realm.kotlin.VersionId
import io.realm.kotlin.dynamic.DynamicRealm
import io.realm.kotlin.internal.dynamic.DynamicRealmImpl
import io.realm.kotlin.internal.interop.RealmInterop
Expand Down Expand Up @@ -83,7 +84,8 @@ public class RealmImpl private constructor(
internal val realmStateFlow =
MutableSharedFlow<State>(replay = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST)

private var _realmReference: AtomicRef<FrozenRealmReference?> = atomic(null)
// Holds the initial realm reference until the notifier or writer can provide with a newer snapshot.
clementetb marked this conversation as resolved.
Show resolved Hide resolved
private var _initialRealmReference: AtomicRef<FrozenRealmReference?> = atomic(null)
private val realmReferenceLock = SynchronizableObject()

/**
Expand All @@ -95,12 +97,6 @@ public class RealmImpl private constructor(
override val realmReference: FrozenRealmReference
get() = realmReference()

// TODO Bit of an overkill to have this as we are only catching the initial frozen version.
// Maybe we could just rely on the notifier to issue the initial frozen version, but that
// would require us to sync that up. Didn't address this as we already have a todo on fixing
// constructing the initial frozen version in the initialization of updatableRealm.
private val versionTracker = VersionTracker(this, log)

// Injection point for synchronized Realms. This property should only be used to hold state
// required by synchronized realms. See `SyncedRealmContext` for more details.
public var syncContext: AtomicRef<Any?> = atomic(null)
Expand Down Expand Up @@ -131,8 +127,7 @@ public class RealmImpl private constructor(
}
val (frozenReference, fileCreated) = configuration.openRealm(this@RealmImpl)
realmFileCreated = assetFileCopied || fileCreated
versionTracker.trackAndCloseExpiredReferences(frozenReference)
_realmReference.value = frozenReference
_initialRealmReference.value = frozenReference
configuration.initializeRealmData(this@RealmImpl, realmFileCreated)
}

Expand Down Expand Up @@ -226,39 +221,46 @@ public class RealmImpl private constructor(
return notifier.registerObserver(t)
}

@Suppress("ComplexCondition", "ReturnCount")
public fun realmReference(): FrozenRealmReference {
realmReferenceLock.withLock {
val value1 = _realmReference.value
// We don't consider advancing the version if is is already closed.
value1?.let {
if (it.isClosed()) return it
val initialRealmReference = _initialRealmReference.value
val notifierVersion = notifier.version
clementetb marked this conversation as resolved.
Show resolved Hide resolved
val writerVersion = writer.version
clementetb marked this conversation as resolved.
Show resolved Hide resolved

if (initialRealmReference != null) {
if (initialRealmReference.isClosed()) return initialRealmReference
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we even get here if the Realm is closed? I would assume we had other checks preventing this?


// Consider versions of current realm, notifier and writer to identify if we should
// advance the user facing realms version to a newer frozen snapshot.
val initialRealmVersion = initialRealmReference.version()

// Check if there is a newer version than the initial snapshot available.
if (
(notifierVersion == null || notifierVersion <= initialRealmVersion) &&
(writerVersion == null || writerVersion <= initialRealmVersion)
) return initialRealmReference

// There is a newer snapshot available. Close the initial one manually, from now on
// we rely on the notifier and writer snapshots.
initialRealmReference.close()
_initialRealmReference.value = null
}

// Consider versions of current realm, notifier and writer to identify if we should
// advance the user facing realms version to a newer frozen snapshot.
val version = value1?.version()
val notifierSnapshot = notifier.version
val writerSnapshot = writer.version
// Find whether the notifier or writer has the latest snapshot.
val newest: LiveRealmHolder<LiveRealm> =
if (writerVersion != null && (notifierVersion == null || writerVersion > notifierVersion))
writer
else
notifier

var newest: LiveRealmHolder<LiveRealm>? = null
if (notifierSnapshot != null && version != null && notifierSnapshot > version) {
newest = notifier
}
@Suppress("ComplexCondition")
if (writerSnapshot != null && version != null && ((writerSnapshot > version) || (notifierSnapshot != null && writerSnapshot > notifierSnapshot))) {
newest = writer
}
if (newest != null) {
_realmReference.value = newest.snapshot
log.debug("$this ADVANCING $version -> ${_realmReference.value?.version()}")
}
return newest.snapshot ?: sdkError("Accessing realmReference before realm has been opened")
}
return _realmReference.value ?: sdkError("Accessing realmReference before realm has been opened")
}

public fun activeVersions(): VersionInfo {
val mainVersions: VersionData? = _realmReference.value?.let { VersionData(it.uncheckedVersion(), versionTracker.versions()) }
return VersionInfo(mainVersions, notifier.versions(), writer.versions())
val initialVersion: VersionId? = _initialRealmReference.value?.uncheckedVersion()
return VersionInfo(initialVersion, notifier.versions(), writer.versions())
}

override fun close() {
Expand All @@ -267,10 +269,12 @@ public class RealmImpl private constructor(
writer.checkInTransaction("Cannot close the Realm while inside a transaction block")
runBlocking {
realmPointerMutex.withLock {
_initialRealmReference.value?.let { reference ->
if (!reference.isClosed()) reference.close()
}
writer.close()
realmScope.cancel()
notifier.close()
versionTracker.close()
// The local realmReference is pointing to a realm reference managed by either the
// version tracker, writer or notifier, so it is already closed
super.close()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,17 @@ import io.realm.kotlin.VersionId
* Version meta data for an overall [Realm]-instance with [VersionData] for the user-facing [Realm]
* and the underlying [SuspendableNotifier]'s and [SuspendableWriter]'s live realms.
*/
public data class VersionInfo(val main: VersionData?, val notifier: VersionData?, val writer: VersionData?) {
val all: Set<VersionId> = setOf(main, notifier, writer).mapNotNull { it?.versions }.flatten().toSet()
val allTracked: Set<VersionId> = setOf(main, notifier, writer).mapNotNull { it?.active }.flatten().toSet()
public data class VersionInfo(
val initialVersion: VersionId?,
val notifier: VersionData?,
val writer: VersionData?,
) {
val all: Set<VersionId> = (
setOf(notifier, writer).mapNotNull { it?.versions }
.flatten() + listOfNotNull(initialVersion)
).toSet()
val allTracked: Set<VersionId> = (
setOf(notifier, writer).mapNotNull { it?.active }
.flatten() + listOfNotNull(initialVersion)
).toSet()
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ class VersionTrackingTests {
// Until we actually query the object
realm.query<Sample>().find()
realm.activeVersions().run {
assertEquals(2, allTracked.size, toString())
assertEquals(1, allTracked.size, toString())
assertNotNull(writer, toString())
assertEquals(1, writer?.active?.size, toString())
assertEquals(0, writer?.active?.size, toString())
}
}

Expand Down