[core] Jadx: ConcurrentModificationException

[bagipro]

Hey,

I've noticed an error when running decompilation in multiple threads in https://github.com/skylot/jadx/blob/master/jadx-core/src/main/java/jadx/core/ProcessClass.java#L97:

ERROR - Failed to decompile class: obfuscated.dgm
jadx.core.utils.exceptions.JadxRuntimeException: Failed to generate code for class: obfuscated.dgm
	at jadx.core.ProcessClass.generateCode(ProcessClass.java:120)
	at jadx.core.dex.nodes.ClassNode.decompile(ClassNode.java:352)
	at jadx.core.dex.nodes.ClassNode.decompile(ClassNode.java:298)
	at jadx.api.JavaClass.decompile(JavaClass.java:59)
	at jadx.gui.jobs.DecompileTask.lambda$scheduleJobs$0(DecompileTask.java:62)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
	at java.base/java.lang.Thread.run(Thread.java:832)
Caused by: java.util.ConcurrentModificationException: null
	at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1013)
	at java.base/java.util.ArrayList$Itr.next(ArrayList.java:967)
	at java.base/java.util.AbstractCollection.addAll(AbstractCollection.java:335)
	at java.base/java.util.HashSet.<init>(HashSet.java:121)
	at jadx.core.ProcessClass.generateCode(ProcessClass.java:97)
	... 7 common frames omitted

Probably there's no reason to attach the APK

[jpstotz]

So the ConcurrentModificationException happens while iterating over ClassNode.getUseIn(). The List this mtehod returns is pretty often iterated in a similar way, so synchronizing access would be pretty hard.

A better approach seems to be to eliminate the modification. From my observations one modification happens in ListUtils.safeReplace triggered by Inlinemethods.replaceClsUsage.

So one problem are the two lines in ListUtils.safeReplace:

list.remove(oldObj);
list.add(newObj);

list.add increases the modification counter and thus can trigger the ConcurrentModificationException. Changing add to set should eliminate that modification so assuming that oldObj is in the list this should work:

int i = list.indexOf(oldObj);
list.set(i, newObj);

@bagipro How reproducible is the error on your system? Does it occur often/every time or only very rarely?

[bagipro]

@jpstotz
I don't think that this is an often issue because I met it the first time

[skylot]

@bagipro I commit a fix, hope it doesn't break anything else 😄

@jpstotz thanks for suggestion, I use it in my fix, but I go further and move detection of cross dependencies to pre-decompilation stage, so now class processing don't rely on changing data 👍