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

(ts) Runtime de-globalling (step 3) #6758

Draft
wants to merge 32 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
3afe810
modernize dev-shell
Nov 6, 2020
a908f48
tweaks
Nov 6, 2020
a093036
law of Demeter
Nov 6, 2020
193c1a7
remove storageService
Nov 12, 2020
4dad548
cleanups
Nov 12, 2020
09fd6b6
surgically remove static (global) runtime object
Nov 17, 2020
32e20ae
clean ups
Nov 18, 2020
1987f50
big runtime usage refactor
Nov 20, 2020
16a0961
cleanups
Nov 20, 2020
ab675d4
cleanups
Nov 20, 2020
328ac92
serious degloballing
Nov 26, 2020
4978ea0
repairs after unpleasant rebase
Dec 8, 2020
1900fe2
remove debugger statement
Dec 8, 2020
00505f1
sleuthing test failures
Dec 8, 2020
6a9f14d
Don't call idle immediately after closing StorageEndpoint
jblebrun Dec 8, 2020
9482eef
Refactors several classes to use injected StorageKeyManager instances…
csilvestrini Dec 8, 2020
7f2ea17
rename EntitySpecTest.kt to javatests/arcs/sdk/GeneratedEntityTest.kt
galganif Dec 8, 2020
7f5e9f6
Fix typo
csilvestrini Dec 8, 2020
2aff690
Tests for BaseHandle.kt
Dec 8, 2020
bdfdce5
Add missing strict deps for type aliases.
arcs-c3po Dec 8, 2020
b4bb177
rename ReferenceSpecTest.kt to javatests/arcs/sdk/GeneratedReferenceT…
galganif Dec 8, 2020
7ebf1f0
add support for inline schema field inside ordered list in fromLitera…
mariakleiner Dec 8, 2020
b81fbd2
Rename EntityHandleManager to HandleManagerImpl to reflect the actual…
yuangu-google Dec 8, 2020
d5242e8
Add more unit tests for DirectStore.
yuangu-google Dec 8, 2020
f1092db
Caching RawEntity hashCode to improve overhead.
alxmrs Dec 9, 2020
408388c
Add error message for CollectionHandle init (#6690)
SHeimlich Dec 9, 2020
0557c0b
Refactor mock helper methods from the Handle tests into testutil/
Dec 9, 2020
be944aa
Remove unused methods from EntityHandleManager and StorageProxy.
Dec 9, 2020
0e2bf53
Create showcase & showcase test for queries
Dec 9, 2020
c9f4166
Disable import ordering in ktlint (#6749)
csilvestrini Dec 10, 2020
75c791e
remove dead code
Dec 10, 2020
4b6bc47
Merge branch 'master' of github.com:PolymerLabs/arcs into dec-dev-she…
Dec 10, 2020
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
Prev Previous commit
Next Next commit
Don't call idle immediately after closing StorageEndpoint
Unbinding from an Android service is only an indication that the client no
longer wants connected/disconnected events from the storage service, and that
from the client's point of view, it is OK for the service to shut down.

But it *doesn't* make any acquired bindings stop work. This means that we
called close, and then called idle, which succeeds, since the binding still
works.

The `idle` call coming in after `onUnbind` was causing a store to get
re-created (and then never cleaned up, since the binding in question would
never be bound/unbound again).

PiperOrigin-RevId: 346218079
  • Loading branch information
jblebrun authored and Scott J. Miles committed Dec 10, 2020
commit 6a9f14d52f9442b990466faf2b0b091eb6bca429
1 change: 0 additions & 1 deletion java/arcs/core/storage/StorageProxyImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ class StorageProxyImpl<Data : CrdtData, Op : CrdtOperationAtTime, T> private con
// Again, if it takes too long, cancel the job.
val storeCloseJob = scheduler.scope.launch {
store.close()
store.idle()
}

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import arcs.sdk.android.storage.service.BindHelper
import arcs.sdk.android.storage.service.StorageService
import arcs.sdk.android.storage.service.StorageServiceIntentHelpers
import arcs.sdk.android.storage.service.bindForIntent
import kotlinx.atomicfu.atomic
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.CoroutineStart
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand Down Expand Up @@ -78,7 +79,13 @@ class AndroidStorageEndpoint<Data : CrdtData, Op : CrdtOperationAtTime, T> const

private val log = TaggedLog { "AndroidStorageEndpoint" }

private val closed = atomic(false)

override suspend fun idle() {
if (closed.value) {
// TODO(b/175070424) Crash here rather than just logging.
log.warning { "idle called after close" }
}
log.debug { "Waiting for service store to be idle" }
outgoingMessagesCount.flow.first { it == 0 }
suspendForResultCallback { resultCallback ->
Expand All @@ -88,6 +95,10 @@ class AndroidStorageEndpoint<Data : CrdtData, Op : CrdtOperationAtTime, T> const
}

override suspend fun onProxyMessage(message: ProxyMessage<Data, Op, T>) {
if (closed.value) {
// TODO(b/175070424) Crash here rather than just logging.
log.warning { "onProxyMessage called after close" }
}
outgoingMessagesCount.increment()
try {
suspendForResultCallback { resultCallback ->
Expand All @@ -105,6 +116,7 @@ class AndroidStorageEndpoint<Data : CrdtData, Op : CrdtOperationAtTime, T> const
}

override suspend fun close() {
closed.value = true
suspendForResultCallback { resultCallback ->
service.unregisterCallback(channelId, resultCallback)
}
Expand Down