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

Catch possible exceptions when the receiver is already unregistered #7947

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

canyie
Copy link
Collaborator

@canyie canyie commented Mar 29, 2024

https://appcenter.ms/users/vvb2060/apps/Magisk/crashes/errors/411818780u/overview

java.lang.IllegalArgumentException: Receiver not registered: a.q@e4bb01c
android.app.LoadedApk.forgetReceiverDispatcher LoadedApk.java:1687
android.app.ContextImpl.unregisterReceiver ContextImpl.java:1840
android.content.ContextWrapper.unregisterReceiver ContextWrapper.java:812
com.topjohnwu.magisk.utils.APKInstall$InstallReceiver.onSuccess APKInstall.java
com.topjohnwu.magisk.utils.APKInstall$InstallReceiver.onReceive APKInstall.java
android.app.LoadedApk$ReceiverDispatcher$Args.broadcastReceiveReg LoadedApk.java:1878
android.app.LoadedApk$ReceiverDispatcher$Args.lambda$getRunnable$0$android-app-LoadedApk$ReceiverDispatcher$Args LoadedApk.java:1816
android.app.LoadedApk$ReceiverDispatcher$Args$$ExternalSyntheticLambda1.run LoadedApk.java:10
android.os.Handler.handleCallback Handler.java:942
android.os.Handler.dispatchMessage Handler.java:99
android.os.Looper.loopOnce Looper.java:242
android.os.Looper.loop Looper.java:374
android.app.ActivityThread.main ActivityThread.java:8911
java.lang.reflect.Method.invoke Method.java:-2
com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run RuntimeInit.java:671
com.android.internal.os.ZygoteInit.main ZygoteInit.java:951

This seems to be caused by a race condition that two broadcasts can be delivered in a short time, and the second broadcast will be handled while the receiver is already unregistered when handling the first broadcast.
TODO: Shall we also prevent the callbacks being ran twice?

@vvb2060
Copy link
Collaborator

vvb2060 commented Mar 29, 2024

Why does this cause a race? I don't think ignoring it is the right thing to do.

@canyie
Copy link
Collaborator Author

canyie commented Mar 30, 2024

Why does this cause a race? I don't think ignoring it is the right thing to do.

var receiver = new InstallReceiver(pkg, onSuccess, onFailure);
context = context.getApplicationContext();
if (pkg != null) {
// If pkg is not null, look for package added event
var filter = new IntentFilter(Intent.ACTION_PACKAGE_ADDED);
filter.addDataScheme("package");
registerReceiver(context, receiver, filter);
}
registerReceiver(context, receiver, new IntentFilter(receiver.sessionId));

@topjohnwu topjohnwu force-pushed the master branch 4 times, most recently from ec54aed to f61827c Compare May 9, 2024 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants