Fixed
Status Update
Comments
se...@google.com <se...@google.com> #2
what is in update.zip ? can you share that file to triage this issue
yb...@google.com <yb...@google.com> #3
MHC19Q for Nexus 5X (bullhead) - https://dl.google.com/dl/android/aosp//bullhead-ota-mhc19q-8fe67a2b.zip
N4F26T for Nexus 5X (bullhead) -https://dl.google.com/dl/android/aosp/bullhead-ota-n4f26t-648ce802.zip
WW-12.2.5.23 for Asus ZenFone Go (ASUS_X014D) -http://dlcdnet.asus.com/pub/ASUS/ZenFone/ZB452KG/UL-ASUS_X014D-WW-12.2.5.23-user.zip
Look like, this happens only with a big images.
N4F26T for Nexus 5X (bullhead) -
WW-12.2.5.23 for Asus ZenFone Go (ASUS_X014D) -
Look like, this happens only with a big images.
yb...@google.com <yb...@google.com> #4
On Windows 10 (x64)
yb...@google.com <yb...@google.com> #5
related to (and probably worked around for now by) internal bug http://b/36046324
i do think we should also start shipping a 64-bit Windows platform tools package.
we should also consider changing android::base::ReadFdToString to call fstat and pre-size the vector. not worthwhile for reading things like /proc/uptime, but the default expansion behavior for a huge file like a full OTA update is going to result in substantial overhead. (which will explain the regression between .3 and .4, since the former will have allocated exactly the necessary number of bytes.)
lastly (and maybe not worth doing at all, depending on how soon we can get folks to use a 64-bit platform tools on Windows, since all the other platforms are already 64-bit-only), we could consider rewriting the adb sideload code to (a) mmap/munmap rather than actually read in to physical memory or (b) pread. i'm not sure what the performance impact of pread would be (especially on Windows where there is no pread equivalent).
i do think we should also start shipping a 64-bit Windows platform tools package.
we should also consider changing android::base::ReadFdToString to call fstat and pre-size the vector. not worthwhile for reading things like /proc/uptime, but the default expansion behavior for a huge file like a full OTA update is going to result in substantial overhead. (which will explain the regression between .3 and .4, since the former will have allocated exactly the necessary number of bytes.)
lastly (and maybe not worth doing at all, depending on how soon we can get folks to use a 64-bit platform tools on Windows, since all the other platforms are already 64-bit-only), we could consider rewriting the adb sideload code to (a) mmap/munmap rather than actually read in to physical memory or (b) pread. i'm not sure what the performance impact of pread would be (especially on Windows where there is no pread equivalent).
da...@gmail.com <da...@gmail.com> #6
Correction to #5: Windows does support pread() - it's in ReadFile()'s OVERLAPPED parameter.
yb...@google.com <yb...@google.com> #7
@7: we'd still need to write the pread wrapper, though. (similar if we went with mmap/munmap.)
https://android-review.googlesource.com/#/c/355481/ pre-sizes the vector, which should be enough to undo the regression from '.3 to '.4...
Description
Version used: 2.2.0-alpha01
Devices/Android versions reproduced on: Android 9
I'm having some random crashes due to CoroutineLiveData
```
Fatal Exception: java.lang.IllegalArgumentException: This source was already added with the different observer
at androidx.lifecycle.MediatorLiveData.addSource + 89(MediatorLiveData.java:89)
at androidx.lifecycle.CoroutineLiveDataKt.addDisposableSource + 102(CoroutineLiveDataKt.java:102)
at androidx.lifecycle.CoroutineLiveData.emitSource$lifecycle_livedata_ktx_release + 200(CoroutineLiveData.java:200)
at androidx.lifecycle.LiveDataScopeImpl$emitSource$2.invokeSuspend + 89(LiveDataScopeImpl.java:89)
at androidx.lifecycle.LiveDataScopeImpl$emitSource$2.invoke(LiveDataScopeImpl.java:11)
at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn + 91(UndispatchedKt.java:91)
at kotlinx.coroutines.BuildersKt__Builders_commonKt.withContext + 156(BuildersKt__Builders_commonKt.java:156)
at kotlinx.coroutines.BuildersKt.withContext + 1(BuildersKt.java:1)
at androidx.lifecycle.LiveDataScopeImpl.emitSource + 88(LiveDataScopeImpl.java:88)
at com.geekorum.ttrss.articles_list.FeedsViewModel$refreshed$1.invokeSuspend + 90(FeedsViewModel.java:90)
at com.geekorum.ttrss.articles_list.FeedsViewModel$refreshed$1.invoke(FeedsViewModel.java:8)
at androidx.lifecycle.BlockRunner$maybeRun$1.invokeSuspend + 147(BlockRunner.java:147)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith + 33(BaseContinuationImpl.java:33)
at kotlinx.coroutines.DispatchedTask.run + 241(DispatchedTask.java:241)
at android.os.Handler.handleCallback + 873(Handler.java:873)
at android.os.Handler.dispatchMessage + 99(Handler.java:99)
at android.os.Looper.loop + 193(Looper.java:193)
at android.app.ActivityThread.main + 6898(ActivityThread.java:6898)
at java.lang.reflect.Method.invoke(Method.java)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run + 537(RuntimeInit.java:537)
at com.android.internal.os.ZygoteInit.main + 858(ZygoteInit.java:858)
```
From what I understand of CoroutineLiveData source code, when calling `LiveDataScope.emitSource()` the previous source is removed before adding the new one on MediatorLiveData.
```
@MainThread
internal fun emitSource(source: LiveData<T>): DisposableHandle {
clearSource()
val newSource = addDisposableSource(source)
emittedSource = newSource
return newSource
}
```
However the removing is done by launching a new coroutine
```
internal fun <T> MediatorLiveData<T>.addDisposableSource(
source: LiveData<T>
): DisposableHandle {
val disposed = AtomicBoolean(false)
addSource(source) {
if (!disposed.get()) {
value = it
} else {
removeSource(source)
}
}
return object : DisposableHandle {
override fun dispose() {
if (disposed.compareAndSet(false, true)) {
CoroutineScope(Dispatchers.Main).launch {
removeSource(source)
}
}
}
}
}
```
So there is no guarantee that `MediatorLiveData.addSource()` will be called after the removing coroutine is executed. This leads to the IllegalArgumentException in `MediatorLiveData.addSource()`
```
@MainThread
public <S> void addSource(@NonNull LiveData<S> source, @NonNull Observer<? super S> onChanged) {
Source<S> e = new Source<>(source, onChanged);
Source<?> existing = mSources.putIfAbsent(source, e);
if (existing != null && existing.mObserver != onChanged) {
throw new IllegalArgumentException(
"This source was already added with the different observer");
}
if (existing != null) {
return;
}
if (hasActiveObservers()) {
e.plug();
}
}
```