Returning view of method list to avoid CME

[CirQ]

The modified method getMethodsByNameAndParamCount is used (and only used) in the FastHierarchy.getSignaturePolymorphicMethod for resolving polymorphic methods. The for-loop will return the method list's iterator, but the list itself can be modified by another body pack transform thread, causing a ConcurrentModificationException. The patch fix the issue by returning a copy of this list.

This patch also fixes the issue #1811 , and a similar issue is #1199 (fixed by #1886 ).

[StevenArzt]

The proposed fix for the multi-threading problem doesn't resolve the root cause. The copy constructor of the lists iterates over the base collection and adds each element. Your fix reduces the time during which a concurrent modification can occur to the time of copying the list. That's for sure muchsorter than the runtime of the transformer itself, but if you're unlucky, you can still hit a ConcurrentModificationException if the list is modified while the copy constructor is running.

I am slightly reluctant to accept an incomplete fix that - on the other hand - migth cost us some performance because the FastHierarchy is called many times.

[CirQ]

The proposed fix for the multi-threading problem doesn't resolve the root cause. The copy constructor of the lists iterates over the base collection and adds each element. Your fix reduces the time during which a concurrent modification can occur to the time of copying the list. That's for sure muchsorter than the runtime of the transformer itself, but if you're unlucky, you can still hit a ConcurrentModificationException if the list is modified while the copy constructor is running.

I am slightly reluctant to accept an incomplete fix that - on the other hand - migth cost us some performance because the FastHierarchy is called many times.

I see. I encountered this issue in an Android analysis project, while in the experiment, a large amount of apks fail due to CME at that site. This patch works fine for these apks.

Anyway, is there any better solution? I found the list itself is a wrapped synchronized collection (https://github.com/soot-oss/soot/blob/develop/src/main/java/soot/SootClass.java#L680), while the iterator returned by this wrapper does not guarantee thread-safety. JDK officially requires synchronization on the iterating site (https://stackoverflow.com/a/1775738). Will this solution be better? If so, I will create a new PR for that.

[StevenArzt]

We should first understand why one thread is searching for a method in a class in which another thread is still creating methods. Let's assume the thread that queries the FastHierarchy runs first. In that case, we don't find the method, because the second threat hasn't created it yet. Let's assume the other way round: We get the method, because the other thread happened to be faster. Does this mean that our results are dependent on thread scheduling, i.e., luck?

My guess would be that we're dealing with phantom methods which are created when resolving a method reference to a callee that doesn't exist. If that is correct, the FastHierarchy would return cast-incompatibility anyway.