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

[jadx-gui] Searching for resources while decompilation is in progress causes various thread synchronization problems #1648

Closed
jpstotz opened this issue Aug 18, 2022 · 5 comments
Assignees
Labels
bug GUI Issues in jadx-gui module resources
Milestone

Comments

@jpstotz
Copy link
Collaborator

jpstotz commented Aug 18, 2022

  1. Open a large APK with "Autostart background decompilation" disabled (e.g. the binary linked in #1631)
  2. Open search dialog
  3. Disable Code search, enable Resource search
  4. Enter some characters in search text field to start decompilation and continue slow typing every few seconds
java.util.ConcurrentModificationException: null
	at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1043)
	at java.base/java.util.ArrayList$Itr.next(ArrayList.java:997)
	at jadx.gui.treemodel.JResource.update(JResource.java:87)
	at jadx.gui.treemodel.JResource.loadNode(JResource.java:97)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:124)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:131)
	at jadx.gui.search.providers.ResourceSearchProvider.load(ResourceSearchProvider.java:112)
	at jadx.gui.search.providers.ResourceSearchProvider.next(ResourceSearchProvider.java:54)
	at jadx.gui.search.SearchJob.run(SearchJob.java:23)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
ERROR - Error load resource node: pdf
java.lang.ArrayIndexOutOfBoundsException: 0 >= 0
	at java.base/java.util.Vector.removeElementAt(Vector.java:580)
	at java.desktop/javax.swing.tree.DefaultMutableTreeNode.remove(DefaultMutableTreeNode.java:207)
	at java.desktop/javax.swing.tree.DefaultMutableTreeNode.removeAllChildren(DefaultMutableTreeNode.java:393)
	at jadx.gui.treemodel.JResource.update(JResource.java:80)
	at jadx.gui.treemodel.JResource.loadNode(JResource.java:97)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:124)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:131)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:131)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:131)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:131)
	at jadx.gui.search.providers.ResourceSearchProvider.load(ResourceSearchProvider.java:112)
	at jadx.gui.search.providers.ResourceSearchProvider.next(ResourceSearchProvider.java:54)
	at jadx.gui.search.SearchJob.run(SearchJob.java:23)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
ERROR - Error load resource node: lib
java.lang.ArrayIndexOutOfBoundsException: 42 >= 42
	at java.base/java.util.Vector.removeElementAt(Vector.java:580)
	at java.desktop/javax.swing.tree.DefaultMutableTreeNode.remove(DefaultMutableTreeNode.java:207)
	at java.desktop/javax.swing.tree.DefaultMutableTreeNode.removeAllChildren(DefaultMutableTreeNode.java:393)
	at jadx.gui.treemodel.JResource.update(JResource.java:80)
	at jadx.gui.treemodel.JResource.update(JResource.java:88)
	at jadx.gui.treemodel.JResource.loadNode(JResource.java:97)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:124)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:131)
	at jadx.gui.search.providers.ResourceSearchProvider.load(ResourceSearchProvider.java:112)
	at jadx.gui.search.providers.ResourceSearchProvider.next(ResourceSearchProvider.java:54)
	at jadx.gui.search.SearchJob.run(SearchJob.java:23)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
WARN  - UI thread interrupted
java.lang.InterruptedException: null
	at java.base/java.lang.Object.wait(Native Method)
	at java.base/java.lang.Object.wait(Object.java:328)
	at java.desktop/java.awt.EventQueue.invokeAndWait(EventQueue.java:1361)
	at java.desktop/java.awt.EventQueue.invokeAndWait(EventQueue.java:1342)
	at java.desktop/javax.swing.SwingUtilities.invokeAndWait(SwingUtilities.java:1480)
	at jadx.gui.utils.UiUtils.uiRunAndWait(UiUtils.java:376)
	at jadx.gui.jobs.BackgroundExecutor$TaskWorker.doInBackground(BackgroundExecutor.java:147)
	at jadx.gui.jobs.BackgroundExecutor$TaskWorker.doInBackground(BackgroundExecutor.java:113)
	at java.desktop/javax.swing.SwingWorker$1.call(SwingWorker.java:304)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.desktop/javax.swing.SwingWorker.run(SwingWorker.java:343)
	at jadx.gui.jobs.BackgroundExecutor.lambda$execute$0(BackgroundExecutor.java:58)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
ERROR - Error load resource node: layout
java.lang.ArrayIndexOutOfBoundsException: 1357 >= 1356
	at java.base/java.util.Vector.elementAt(Vector.java:497)
	at java.desktop/javax.swing.tree.DefaultMutableTreeNode.getChildAt(DefaultMutableTreeNode.java:246)
	at java.desktop/javax.swing.tree.DefaultMutableTreeNode.remove(DefaultMutableTreeNode.java:206)
	at java.desktop/javax.swing.tree.DefaultMutableTreeNode.removeAllChildren(DefaultMutableTreeNode.java:393)
	at jadx.gui.treemodel.JResource.update(JResource.java:80)
	at jadx.gui.treemodel.JResource.loadNode(JResource.java:97)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:124)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:131)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:131)
	at jadx.gui.search.providers.ResourceSearchProvider.load(ResourceSearchProvider.java:112)
	at jadx.gui.search.providers.ResourceSearchProvider.next(ResourceSearchProvider.java:54)
	at jadx.gui.search.SearchJob.run(SearchJob.java:23)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
ERROR - Error load resource node: resources.arsc
java.lang.ArrayIndexOutOfBoundsException: 15 >= 15
	at java.base/java.util.Vector.elementAt(Vector.java:497)
	at java.desktop/javax.swing.tree.DefaultMutableTreeNode.getChildAt(DefaultMutableTreeNode.java:246)
	at java.desktop/javax.swing.tree.DefaultMutableTreeNode.remove(DefaultMutableTreeNode.java:206)
	at java.desktop/javax.swing.tree.DefaultMutableTreeNode.removeAllChildren(DefaultMutableTreeNode.java:393)
	at jadx.gui.treemodel.JResource.update(JResource.java:80)
	at jadx.gui.treemodel.JResource.loadNode(JResource.java:97)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:124)
	at jadx.gui.search.providers.ResourceSearchProvider.traverseTree(ResourceSearchProvider.java:131)
	at jadx.gui.search.providers.ResourceSearchProvider.load(ResourceSearchProvider.java:112)
	at jadx.gui.search.providers.ResourceSearchProvider.next(ResourceSearchProvider.java:54)
	at jadx.gui.search.SearchJob.run(SearchJob.java:23)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

@jpstotz jpstotz added bug Core Issues in jadx-core module labels Aug 18, 2022
@skylot
Copy link
Owner

skylot commented Aug 18, 2022

@jpstotz thanks!
Looks like I need to do a heavy refactoring for this resource search, main issues:

  • load method is synchronized only for current instance, but every new search create a new instance
  • load method is not cancelable, so it doesn't stop on search cancel
  • also it is not really needed to preload anything, it is better to adapt iterations over resources tree and load nodes on demand

@skylot skylot self-assigned this Aug 18, 2022
@skylot skylot added GUI Issues in jadx-gui module resources and removed Core Issues in jadx-core module labels Aug 18, 2022
@skylot skylot added this to the TBD milestone Aug 18, 2022
@skylot
Copy link
Owner

skylot commented Aug 19, 2022

I commit changes I mention above. Can anyone check it?

@jpstotz
Copy link
Collaborator Author

jpstotz commented Aug 19, 2022

@skylot Thanks for the fix.

The search dialog seem to be now safe in 99.99% of all cases.

One time I got the following stack trace, but no matter how hard I tried I wasn't able to reproduce it:

WARN  - Search error, provider: ResourceSearchProvider
java.util.NoSuchElementException: Vector Enumeration
	at java.base/java.util.Vector$1.nextElement(Vector.java:375)
	at jadx.gui.search.providers.ResourceSearchProvider.addChildren(ResourceSearchProvider.java:121)
	at jadx.gui.search.providers.ResourceSearchProvider.getNextNode(ResourceSearchProvider.java:114)
	at jadx.gui.search.providers.ResourceSearchProvider.getNextNode(ResourceSearchProvider.java:110) [repeated 130 times]
	at jadx.gui.search.providers.ResourceSearchProvider.getNextNode(ResourceSearchProvider.java:115)
	at jadx.gui.search.providers.ResourceSearchProvider.getNextNode(ResourceSearchProvider.java:110) [repeated 117 times]
	at jadx.gui.search.providers.ResourceSearchProvider.next(ResourceSearchProvider.java:55)
	at jadx.gui.search.SearchJob.run(SearchJob.java:23)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

After that I only noticed one minor issue in combination with UiUtils.uiRunAndWait which prints the stack trace when interrupted. I managed to trigger those one time while playing with the search dialog in a pretty "wild way":

WARN  - UI thread interrupted
java.lang.InterruptedException: null
	at java.base/java.lang.Object.wait(Native Method)
	at java.base/java.lang.Object.wait(Object.java:328)
	at java.desktop/java.awt.EventQueue.invokeAndWait(EventQueue.java:1361)
	at java.desktop/java.awt.EventQueue.invokeAndWait(EventQueue.java:1342)
	at java.desktop/javax.swing.SwingUtilities.invokeAndWait(SwingUtilities.java:1480)
	at jadx.gui.utils.UiUtils.uiRunAndWait(UiUtils.java:376)
	at jadx.gui.jobs.BackgroundExecutor$TaskWorker.doInBackground(BackgroundExecutor.java:147)
	at jadx.gui.jobs.BackgroundExecutor$TaskWorker.doInBackground(BackgroundExecutor.java:113)
	at java.desktop/javax.swing.SwingWorker$1.call(SwingWorker.java:304)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.desktop/javax.swing.SwingWorker.run(SwingWorker.java:343)
	at jadx.gui.jobs.BackgroundExecutor.lambda$execute$0(BackgroundExecutor.java:58)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

But as the task is already completed it not really a problem, just printing the strack trace looks like there is a problem.

@skylot
Copy link
Owner

skylot commented Aug 19, 2022

@jpstotz thanks for check and feedback!
I commit a change to reduce possible concurrent modifications by loading resource table resources in "one change", previously tree was loaded incrementally, so it was hard to synchronize these updates.

WARN - UI thread interrupted

This is just a warning, indicating that search provider checks cancel state not frequently enough, so it can't stop in time (timeout is very strict). This is a good indicator of bad implementation, so I want to keep it. In last change I made an additional check, so it shouldn't show often 🙂.

@jpstotz
Copy link
Collaborator Author

jpstotz commented Aug 20, 2022

@skylot If you prefer to keep the log message with stack trace as it is, I am also fine with this.

Thanks again for the immediate fix - looks like this issue comes it an end...

@jpstotz jpstotz closed this as completed Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug GUI Issues in jadx-gui module resources
Projects
None yet
Development

No branches or pull requests

2 participants