Fixed
Status Update
No update yet.
Comments
ra...@google.com
Can you please send us the unobfuscated stack trace ? Also, can you reproduce this locally ? If so, please send us a sample app.
Can you also try using 2.1.0-alpha02 to see if that helps ?
Can you also try using 2.1.0-alpha02 to see if that helps ?
ra...@google.com
I can see the potential for a deadlock here (because we don't acquire locks in the same order).
d....@googlemail.com
> Can you please send us the unobfuscated stack trace ?
The stack trace is from the Google Play console from a version for which I had uploaded a deobfuscation file. So I think the remaining obfuscation is already contained in the WorkManager library and cannot be unobfuscated by me except by manually searching the correct lines which I did. Behind each line in the stack trace you can see my manual comment to what the line refers to. Or can I download Proguard mapping files for the WorkManager versions?
> Also, can you reproduce this locally ? If so, please send us a sample app.
Unfortunately I cannot reproduce it myself and it seems to require special circumstances. I am trying to wake my app each time the phone is connected to a Wi-Fi network. For this purpose I use the command
WorkManager.getInstance().enqueueUniqueWork(WIFI_DISCONNECT, ExistingWorkPolicy.REPLACE,
OneTimeWorkRequest.Builder(WifiCheckDisconnected::class.java)
.setConstraints(Constraints.Builder()
.setRequiredNetworkType(NetworkType.METERED)
.build())
.setInitialDelay(60, TimeUnit.MINUTES)
.build())
to wait for a disconnect from the Wi-Fi network and then
WorkManager.getInstance().apply {
enqueueUniqueWork(WIFI_CONNECT, ExistingWorkPolicy.REPLACE,
OneTimeWorkRequest.Builder(WifiConnected::class.java)
.setConstraints(Constraints.Builder().setRequiredNetworkType(NetworkType.UNMETERED).build())
.build())
cancelUniqueWork(WIFI_DISCONNECT)
}
to wait for a connection to a Wi-Fi network. It seems that due one of these commands onCapabilitiesChanged is called by the ConnectivityManager. Unfortunately it is called exactly in parallel to another WorkManager command, which causes the deadlock. Maybe the ConnectivityManager calls onCapabilitiesChanged twice quickly after another and the first call causes my program to enqueue the other work task. Because WifiConnected.doWork looks like this:
override fun doWork(): ListenableWorker.Result {
// Wait for Wi-Fi disconnect
WorkManager.getInstance().enqueueUniqueWork(WIFI_DISCONNECT, ExistingWorkPolicy.REPLACE,
OneTimeWorkRequest.Builder(WifiCheckDisconnected::class.java)
.setConstraints(Constraints.Builder().setRequiredNetworkType(NetworkType.METERED).build())
.build())
> Can you also try using 2.1.0-alpha02 to see if that helps ?
I can only reproduce it in the release version of my app when it is used by several thousands of users. So because I cannot reproduce it on my own phone yet I cannot check if 2.1.0-alpha02 helps. Instead I will probably try to use the JobScheduler until this is fixed.
And I think the only way to reproduce it would be to trigger onCapabilitiesChanged somehow programmatically.
> I can see the potential for a deadlock here (because we don't acquire locks in the same order).
Great, I hope you can fix this bug! I like the WorkManager very much but it should not cause any deadlocks. ;-)
Please don't hesitate to contact me again if you have further questions!
The stack trace is from the Google Play console from a version for which I had uploaded a deobfuscation file. So I think the remaining obfuscation is already contained in the WorkManager library and cannot be unobfuscated by me except by manually searching the correct lines which I did. Behind each line in the stack trace you can see my manual comment to what the line refers to. Or can I download Proguard mapping files for the WorkManager versions?
> Also, can you reproduce this locally ? If so, please send us a sample app.
Unfortunately I cannot reproduce it myself and it seems to require special circumstances. I am trying to wake my app each time the phone is connected to a Wi-Fi network. For this purpose I use the command
WorkManager.getInstance().enqueueUniqueWork(WIFI_DISCONNECT, ExistingWorkPolicy.REPLACE,
OneTimeWorkRequest.Builder(WifiCheckDisconnected::class.java)
.setConstraints(Constraints.Builder()
.setRequiredNetworkType(NetworkType.METERED)
.build())
.setInitialDelay(60, TimeUnit.MINUTES)
.build())
to wait for a disconnect from the Wi-Fi network and then
WorkManager.getInstance().apply {
enqueueUniqueWork(WIFI_CONNECT, ExistingWorkPolicy.REPLACE,
OneTimeWorkRequest.Builder(WifiConnected::class.java)
.setConstraints(Constraints.Builder().setRequiredNetworkType(NetworkType.UNMETERED).build())
.build())
cancelUniqueWork(WIFI_DISCONNECT)
}
to wait for a connection to a Wi-Fi network. It seems that due one of these commands onCapabilitiesChanged is called by the ConnectivityManager. Unfortunately it is called exactly in parallel to another WorkManager command, which causes the deadlock. Maybe the ConnectivityManager calls onCapabilitiesChanged twice quickly after another and the first call causes my program to enqueue the other work task. Because WifiConnected.doWork looks like this:
override fun doWork(): ListenableWorker.Result {
// Wait for Wi-Fi disconnect
WorkManager.getInstance().enqueueUniqueWork(WIFI_DISCONNECT, ExistingWorkPolicy.REPLACE,
OneTimeWorkRequest.Builder(WifiCheckDisconnected::class.java)
.setConstraints(Constraints.Builder().setRequiredNetworkType(NetworkType.METERED).build())
.build())
> Can you also try using 2.1.0-alpha02 to see if that helps ?
I can only reproduce it in the release version of my app when it is used by several thousands of users. So because I cannot reproduce it on my own phone yet I cannot check if 2.1.0-alpha02 helps. Instead I will probably try to use the JobScheduler until this is fixed.
And I think the only way to reproduce it would be to trigger onCapabilitiesChanged somehow programmatically.
> I can see the potential for a deadlock here (because we don't acquire locks in the same order).
Great, I hope you can fix this bug! I like the WorkManager very much but it should not cause any deadlocks. ;-)
Please don't hesitate to contact me again if you have further questions!
d....@googlemail.com
Thank you very much for your quick responses! I am glad that you are fixing this bug so fast! Can you please post here in which version/release the changes will be included when you have finished implementing the changes?
Thank you very much! :-)
Thank you very much! :-)
ra...@google.com
This should be available in the next version of WorkManager.
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit 2d2b53f0f39033e72b686d5b56ad8be2e5eec900
Author: Rahul Ravikumar <rahulrav@google.com>
Date: Tue Jun 04 12:42:38 2019
Notify constraint changes on the main thread only.
Fixes:
Test: Ran integration tests. Updated unit tests.
Change-Id: I3e84c5f3229c30e810638ba97fb4ac5414288faa
M work/workmanager-testing/src/main/java/androidx/work/testing/TestWorkManagerImpl.java
M work/workmanager/src/androidTest/java/androidx/work/impl/WorkManagerImplLargeExecutorTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/background/greedy/GreedySchedulerTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/background/systemalarm/SystemAlarmDispatcherTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/constraints/trackers/BatteryChargingTrackerTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/constraints/trackers/BatteryNotLowTrackerTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/constraints/trackers/ConstraintTrackerTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/constraints/trackers/NetworkStateTrackerTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/constraints/trackers/StorageNotLowTrackerTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/workers/ConstraintTrackingWorkerTest.java
M work/workmanager/src/main/java/androidx/work/impl/WorkManagerImpl.java
M work/workmanager/src/main/java/androidx/work/impl/background/greedy/GreedyScheduler.java
M work/workmanager/src/main/java/androidx/work/impl/background/systemalarm/ConstraintsCommandHandler.java
M work/workmanager/src/main/java/androidx/work/impl/background/systemalarm/DelayMetCommandHandler.java
M work/workmanager/src/main/java/androidx/work/impl/background/systemalarm/SystemAlarmDispatcher.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/ConstraintListener.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/WorkConstraintsTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/BatteryChargingController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/BatteryNotLowController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/NetworkConnectedController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/NetworkMeteredController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/NetworkNotRoamingController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/NetworkUnmeteredController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/StorageNotLowController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/BatteryChargingTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/BatteryNotLowTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/BroadcastReceiverConstraintTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/ConstraintTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/NetworkStateTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/StorageNotLowTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/Trackers.java
M work/workmanager/src/main/java/androidx/work/impl/workers/ConstraintTrackingWorker.java
https://android-review.googlesource.com/975608
https://goto.google.com/android-sha1/2d2b53f0f39033e72b686d5b56ad8be2e5eec900
Branch: androidx-master-dev
commit 2d2b53f0f39033e72b686d5b56ad8be2e5eec900
Author: Rahul Ravikumar <rahulrav@google.com>
Date: Tue Jun 04 12:42:38 2019
Notify constraint changes on the main thread only.
Fixes:
Test: Ran integration tests. Updated unit tests.
Change-Id: I3e84c5f3229c30e810638ba97fb4ac5414288faa
M work/workmanager-testing/src/main/java/androidx/work/testing/TestWorkManagerImpl.java
M work/workmanager/src/androidTest/java/androidx/work/impl/WorkManagerImplLargeExecutorTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/background/greedy/GreedySchedulerTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/background/systemalarm/SystemAlarmDispatcherTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/constraints/trackers/BatteryChargingTrackerTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/constraints/trackers/BatteryNotLowTrackerTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/constraints/trackers/ConstraintTrackerTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/constraints/trackers/NetworkStateTrackerTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/constraints/trackers/StorageNotLowTrackerTest.java
M work/workmanager/src/androidTest/java/androidx/work/impl/workers/ConstraintTrackingWorkerTest.java
M work/workmanager/src/main/java/androidx/work/impl/WorkManagerImpl.java
M work/workmanager/src/main/java/androidx/work/impl/background/greedy/GreedyScheduler.java
M work/workmanager/src/main/java/androidx/work/impl/background/systemalarm/ConstraintsCommandHandler.java
M work/workmanager/src/main/java/androidx/work/impl/background/systemalarm/DelayMetCommandHandler.java
M work/workmanager/src/main/java/androidx/work/impl/background/systemalarm/SystemAlarmDispatcher.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/ConstraintListener.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/WorkConstraintsTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/BatteryChargingController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/BatteryNotLowController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/NetworkConnectedController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/NetworkMeteredController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/NetworkNotRoamingController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/NetworkUnmeteredController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/controllers/StorageNotLowController.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/BatteryChargingTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/BatteryNotLowTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/BroadcastReceiverConstraintTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/ConstraintTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/NetworkStateTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/StorageNotLowTracker.java
M work/workmanager/src/main/java/androidx/work/impl/constraints/trackers/Trackers.java
M work/workmanager/src/main/java/androidx/work/impl/workers/ConstraintTrackingWorker.java
Description
Version used: 1.0.1
Devices/Android versions reproduced on: different Samsung Galaxy devices running Android 8 and 9
I have received deadlock reports like this:
"main" prio=5 tid=1 Blocked
| group="main" sCount=1 dsCount=0 flags=1 obj=0x76790a58 self=0x7d81014c00
| sysTid=27874 nice=0 cgrp=default sched=0/0 handle=0x7e07c1a560
| state=S schedstat=( 1835900780 7285545518 5958 ) utm=82 stm=100 core=1 HZ=100
| stack=0x7febcdd000-0x7febcdf000 stackSize=8MB
| held mutexes=
at androidx.work.impl.constraints.trackers.ConstraintTracker.removeListener (ConstraintTracker.java:77)
- waiting to lock <0x08a4f10d> (a java.lang.Object) held by thread 13 // ConstraintTracker.mLock
at androidx.work.impl.constraints.controllers.ConstraintController.replace (ConstraintController.java:98)
at androidx.work.impl.constraints.WorkConstraintsTracker.replace (WorkConstraintsTracker.java:99)
- locked <0x0cbac8c2> (a java.lang.Object) // WorkConstrainsTracker.mLock
at androidx.work.impl.background.greedy.GreedyScheduler.removeConstraintTrackingFor (GreedyScheduler.java:153)
- locked <0x0cbec6d3> (a java.lang.Object) // GreedyScheduler.mLock
at androidx.work.impl.background.greedy.GreedyScheduler.onExecuted (GreedyScheduler.java:141)
at androidx.work.impl.Processor.onExecuted (Processor.java:230)
- locked <0x08012210> (a java.lang.Object)
at androidx.work.impl.Processor$FutureListener.run (Processor.java:263)
at android.os.Handler.handleCallback (Handler.java:873)
at android.os.Handler.dispatchMessage (Handler.java:99)
at android.os.Looper.loop (Looper.java:214)
at android.app.ActivityThread.main (ActivityThread.java:7032)
at java.lang.reflect.Method.invoke (Method.java)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:494)
at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:965)
"ConnectivityThread" tid=13 Blocked
"ConnectivityThread" prio=5 tid=13 Blocked
| group="main" sCount=1 dsCount=0 flags=1 obj=0x12fc0770 self=0x7d7ab86800
| sysTid=27897 nice=0 cgrp=default sched=0/0 handle=0x7d684114f0
| state=S schedstat=( 247567475 11539483672 1368 ) utm=16 stm=7 core=2 HZ=100
| stack=0x7d6830e000-0x7d68310000 stackSize=1041KB
| held mutexes=
at androidx.work.impl.a.d.c (SourceFile:157) // synchronized (mLock)
- waiting to lock <0x0cbac8c2> (a java.lang.Object) held by thread 1 // trying to lock WorkConstraintsTracker.mLock
at androidx.work.impl.a.a.c.b (SourceFile:133) ConstraintController: mCallback.onConstraintNotMet(mMatchingWorkSpecIds);
at androidx.work.impl.a.a.c.a (SourceFile:142) ConstraintController: updateCallback();
at androidx.work.impl.a.b.d.a (SourceFile:103) listener.onConstraintChanged(mCurrentState);
- locked <0x08a4f10d> (a java.lang.Object) (ConstraintTracker.mLock)
at androidx.work.impl.a.b.e$b.onCapabilitiesChanged (SourceFile:149) (NetworkStateTracker)
at android.net.ConnectivityManager$CallbackHandler.handleMessage (ConnectivityManager.java:3331)
at android.os.Handler.dispatchMessage (Handler.java:106)
at android.os.Looper.loop (Looper.java:214)
at android.os.HandlerThread.run (HandlerThread.java:65)
I have written after the lines what is happening there.
The GreedyScheduler has finished executing and onExecuted is called. This tries to remove the workSpecId from the mConstrainedWorkSpecs list. After removing it it calls mWorkConstraintsTracker.replace to send the new list to the WorkContraintsTracker. This causes a lock of WorkConstraintsTracker.mLock and a call to ConstraintController.replace. This calls ConstraintController.removeListener which tries to lock ConstraintController.mLock but fails/deadlocks. So as you can see it has first locked WorkConstraintsTracker.mLock and then it tries to lock ConstraintController.mLock.
As you can see now the other thread does the opposite, which leads to the deadlock: The WorkManager receives a "onCapabilitiesChanges" message and locks ConstraintTracker.mLock. Then it tries to call onConstraintNotMet but it cannot lock WorkConstraintsTracker.mLock, because it is already locked by the upper thread.
So there is a deadlock because both functions are running at the same time and are locking the locks in opposite order. Can you please fix it? Or did I make a mistake somewhere?
(BTW I am only using WorkManager.getInstance, I am not calling "initialize").
Thank you very much!