search

soot-oss / soot

Soot - A Java optimization framework
GNU Lesser General Public License v2.1
2.84k stars 708 forks source link

fix ConcurrentModificationException when calling SootClass.getMethods… #2043

Open buriedpot opened 6 months ago

buriedpot commented 6 months ago

Issue #2024

StevenArzt commented 6 months ago

Your merge request seems inconsistent with the issue #2024. The synchronization will only help in a multithreaded context, and only if the other operations that modify the collection are also synchronized. I don't see that this is the case. I rather assume that the overhead introduced by the synchronization fixes some weird timing issue by chance. If that's correct, the problem remains. Further, everyone pays for the overhead of synchronization. In total, I'm not really happy with this fix suggestion.

buriedpot commented 6 months ago

Can you review my newest commit fb376b8, thank you!

buriedpot commented 6 months ago

Your merge request seems inconsistent with the issue #2024. The synchronization will only help in a multithreaded context, and only if the other operations that modify the collection are also synchronized. I don't see that this is the case. I rather assume that the overhead introduced by the synchronization fixes some weird timing issue by chance. If that's correct, the problem remains. Further, everyone pays for the overhead of synchronization. In total, I'm not really happy with this fix suggestion.

Can you review my newest commit fb376b8, thank you!

Can you review my newest commit fb376b8, thank you!

StevenArzt commented 5 months ago

The copy constructor internally just iterates over the incoming list. You can just have the ConcurrentModificationException inside the constructor if you're unlucky. A real fix would need to make the data structures concurrent, which is a major undertaking.