igvteam / igv

Integrative Genomics Viewer. Fast, efficient, scalable visualization tool for genomics data and annotations
https://igv.org
MIT License
637 stars 384 forks source link

IGV 2.8.x freezes when receiving commands over a port #859

Closed p69180 closed 3 years ago

p69180 commented 3 years ago

Hi,

IGV version 2.8.x seems to freeze when receiving commands over a port. I send commands to IGV by doing like "echo sort base | nc localhost 60151". Loading a bam file is okay, but then when I enter a command to go to a specific position, IGV freezes. I wait until IGV returns "OK" before I type the next command. This problem does not occur with IGV 2.6.x.

Thank you.

hioymaci commented 3 years ago

Yes. I have also same bug. I run IGV in different OS and different data but all of them freezes IGV. In addition to port mode, in batch mode IGV freezes. I changed version to 2.3.97, IGV runs properly (no freeze).

tomasyeo commented 3 years ago

IGV Linux either crashes or freezes in batch mode. 2.8.0 is intermittent, but getting more crashes lately. 2.8.12 kept freezing up after a few goto.

I can't find the log file anywhere in my home folder or elsewhere. How do capture the log? Thanks.

jrobinso commented 3 years ago

@tomasyeo Technically there is no "batch" mode, IGV requires a gui and is an interactive program. Can you be more specific by what you mean by "batch mode"?

The "igv" directory is created in your home folder. Within that directory is a file called "igv.log".

I'll be investigating this port issue later this week. My test work, so I might need a reproducible test case.

tomasyeo commented 3 years ago

@tomasyeo Technically there is no "batch" mode, IGV requires a gui and is an interactive program. Can you be more specific by what you mean by "batch mode"?

The "igv" directory is created in your home folder. Within that directory is a file called "igv.log".

I'll be investigating this port issue later this week. My test work, so I might need a reproducible test case.

Thanks for the quick response. I don't think the "igv' directory is created at all. I could redirect stdout to a file if there is a java parameter to set log verbosity?

I meant command line option -batch. -b Path or url to a batch command file eg. "igv.sh -b cmds.txt"

My sequence of commands go like this: _new genome reference snapshotDirectory ./snapshot maxPanelHeight 120 load 1.bam load 2.bam load 3.bam load 4.bam squish goto Pf3D7_02_v3:612283-612383 snapshot Pf3D7_02_v3_612333.png goto Pf3D7_03_v3:284095-284195 snapshot Pf3D7_03_v3_284145.png goto Pf3D7_03_v3:706411-706511 snapshot Pf3D7_03_v3706461.png ... exit_

jrobinso commented 3 years ago

@hioymaci @tomasyeo @p69180

BTW the 2.8 branch has reached its end of life, I won't be releasing any more patches there except for trivial bugs. These are not trivial bugs, so any fixes will be in the "nightly snapshot" until the next major release at the end of the year. The master branch is significantly different from 2.8, its possible your issues are already fixed there. In any event its worth trying that build, available as "nightly snapshot". Basically if I can't reproduce the problem(s) there they won't get fixed.

Its really helpful to submit reproducible test cases using URLs to publically available bam files, of which there are many including from our own "Load from server" menu.

@tomasyeo If an igv folder is not created you have some machine specific problem, perhaps permissions? I can't really debug that here obviously, but without an igv folder igv cannot run at all so a folder is created somewhere. It is possible to set the location of the igv folder from preferences ("advanced"), so you might check that tab to see what it says (View > Preferences > Advanced).

jrobinso commented 3 years ago

@p69180 I can't reproduce any issues on the master branch (snapshot release). As an example this works

echo goto egfr | nc localhost 60151

Can you give me a command that reliably freezes IGV, or list of commands. You can use our public bam files from "load from server", or any public file, if needed. If not I'll assume whatever the issue was is fixed in this branch.

jrobinso commented 3 years ago

@tomasyeo I can't run your script obviously, but I can reproduce a problem with goto with this one: https://github.com/igvteam/igv/blob/master/test/data/batch/squish_specific_track.txt. I will work from there.

This issue has gotten confusing, the original issue was commands over a port, not from the command line, for now I'll keep it a single issue but might split it.

jrobinso commented 3 years ago

I've pushed a possible fix for the goto problem specific to bam files (i.e. if bam files are loaded) to the master branch, available through the snapshot build. This fix will likely not be applied to the 2.8.x branch, a 2.9 release should be out by the end of the year, in the meantime please use the snapshot build if you need this. Update this thread if the problem remains, or other problems are encountered.

bbreton commented 3 years ago

Unfortunately I don't think this is fixed. The script below might work better (hard to be certain when failure is random), but it is still freezing.

Removing my ~/igv directory seems to help it work for a bit so I suspect the issue is related to caching somehow. I cleared it when I switched to the latest build so I'm not certain if the ~/igv made it work better or if the new code improved the reliability.

Overall it feels like a thread getting stuck or a race condition that only happens when something is cached given it always works the first time with a clean ~/igv Edit: Tried cleaning the ~/igv again and this time it didn't help so I don't know and seems to work better when other stuff is done inbetween. Unfortunately I really don't know java and threading well enough to try to fix this myself.

When it gets stuck the last trace message is: DEBUG [2020-11-19T13:43:37,134] [MessageUtils.java:107] [pool-3-thread-2] Status bar: Reading...

When it works the next trace message after that one is: TRACE [2020-11-19T13:42:41,681] [IGVSeekableHTTPStream.java:122] [AWT-EventQueue-0] Trying to read range 0 to 131071

If there is any more info you would like let me know and I'll do my best to get it.

Much of my testing including the logs were with me compiling it, but I tested the nightly snapshot on the website downloaded today as well and I saw the same behavior.

The one thing I haven't tested with is trying to run java11. I'm running maco Catalina v 10.15.7.

Full stderr: igv_script_log_bad_run.log igv_script_log_working_run.log

Environment info

Using system JDK. WARNING: package com.sun.java.swing.plaf.windows not in java.desktop WARNING: package sun.awt.windows not in java.desktop openjdk version "14.0.1" 2020-04-14 OpenJDK Runtime Environment (build 14.0.1+7) OpenJDK 64-Bit Server VM (build 14.0.1+7, mixed mode, sharing)

Script Run

#Always works with Tools -> Run Batch Script
#CLI is unreliable: ./igv.sh -b only_works_first_time.script
#Frequently though not all the time fails.  In general doing other stuff with igv e.g. loading genomes, other bam files etc. seem to make it more likely to work.  It fails the majority of the time when run twice in a row.
#Uncommenting the hg38 and making it load hg38 then load hg19 makes it work more often, but not all the time.
#genome hg38
genome hg19
load http://1000genomes.s3.amazonaws.com/phase3/data/HG00268/high_coverage_alignment/HG00268.wgs.ILLUMINA.bwa.FIN.high_cov_pcr_free.20140203.bam
goto chr9:12054933-12055133
exit
jrobinso commented 3 years ago

@bbreton Thanks that's great information. Back to the drawing board. Its extraordinarily hard and hacky to get a multi-threaded Swing app to function in a stictly single-threaded synchronous fashion as required for batch scripting. In the future we might support this through the javascript component with a node app, there are external groups working on that now. In the meantime will keep trying.

RE Java versions we only support long-term-support releases, with is currently Java 11. I don't recall when the next LTS release is but its not for some time. I don't think deploying production systems with non LTS Java versions is recommended, but in any event we aren't going to do it.

I have an igv.js and web-app release to finish and then will be out for a week, but will get back to this as soon as I return.

bbreton commented 3 years ago

Since I noticed that the batch scripting worked properly without freezing from the GUI I gave a shot making the batch scripts get loaded more like that by putting them in the EventThread rather than LongRunningTask.

I'm not sure if this will work for everything, but it appears to work at least for the basics. I'm fairly certain this isn't the proper solution (aka a terrible hack) so I'm not doing a pull request. If you do think this is actually a viable solution and I don't find any issues with it as I run more through it I can do it as one.

Diff from intellij: image

Patch:

Index: src/main/java/org/broad/igv/ui/IGV.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/main/java/org/broad/igv/ui/IGV.java (revision 23fae0a71fb18ce88105fc8b8026c4565c595ec1)
+++ src/main/java/org/broad/igv/ui/IGV.java (date 1605821697236)
@@ -2227,19 +2227,30 @@
                 }
                 CommandListener.start(port);
             }
-
-            UIUtilities.invokeOnEventThread(new Runnable() {
-                public void run() {
-                    mainFrame.setVisible(true);
-                    if (igvArgs.getLocusString() != null) {
-                        goToLocus(igvArgs.getLocusString());
+            if (!runningBatch) {
+                UIUtilities.invokeOnEventThread(new Runnable() {
+                    public void run() {
+                        mainFrame.setVisible(true);
+                        if (igvArgs.getLocusString() != null) {
+                            goToLocus(igvArgs.getLocusString());
+                        }
                     }
-                    if (runningBatch) {
+                });
+            }
+            if (runningBatch) {
+                UIUtilities.invokeOnEventThread(new Runnable() {
+                    public void run() {
+                        mainFrame.setVisible(true);
+                        if (igvArgs.getLocusString() != null) {
+                            goToLocus(igvArgs.getLocusString());
+                        }
                         Globals.setBatch(false);   // Set to false for startup execution -- otherwise we block the event thread
-                        LongRunningTask.submit(new BatchRunner(igvArgs.getBatchFile(), IGV.this));
+                        final BatchRunner bRun = new BatchRunner(igvArgs.getBatchFile(), IGV.this);
+                        bRun.run();
                     }
-                }
-            });
+                });
+            }
+

             synchronized (IGV.getInstance()) {
                 IGV.getInstance().notifyAll();
bbreton commented 3 years ago

So for that code chunk I posted... I really doesn't work. It did kind of work for some really simple stuff and at least it doesn't freeze, but the second you switch bamfiles or do a variety of other stuff the painting doesn't happen reliably so random parts of the GUI don't get updated.

Since I noticed 3.8.2 batch scripting is stable when triggering from the GUI with Run Batch Script I checked out the IGV 3.8.2 git tag, Added a keyboard shortcut to trigger a hardcoded batchscript path using the same codepath as Run Batch Script and used xdotools (linux utility for automating xwindows) to send the keyboard shortcut and trigger the keyboard shortcut making it follow the 'GUI' codepath and threading for running the script rather than the 'batch' mode.

This was a terrible terrible hack, but it worked the best. That actually may be the best 'real' solution for batch scripting is make it follow the same codepath fully as the gui one just in a less hacky way than I did (mostly since I don't know java/the codebase well enough to figure out how to trigger it properly). It also would mean only 1 path needs testing.

jrobinso commented 3 years ago

@bbreton I'm confused, we don't have a "3.8.2" tag, so I'm not sure what you are running. The relevant code in "master" is significantly different that the 2.8 branch and is what we should concentrate on. I have fixed a couple of bugs there related to this, and as far as I know it works, but if it doesn't let me know. Again nothing is going to get fixed in the 2.8 branch other than blocking bugs for normal (i.e. UI) usage.

jrobinso commented 3 years ago

@bbreton Thanks for the insight on the GUI path, I will investigate the implications of that. Running everything on the event thread would make it single threaded, which is the intent of the currrent implementation in the "master" branch, but perhaps some branch is missed.

jrobinso commented 3 years ago

@bbreton Following up on your observation that the batch script always works from the GUI thread, I made a simple change to just execute BatchRunner on the Swing thread. Intuitively this is wrong, but that is what the GUI batch option has always done. Your test script worked reliably for me, but it worked most of the time before. I didn't push this to the master branch, I want to figure out why it works, but I did push to branch "batch_event_thread". You can pull and test that one if you like.

ConorMesser commented 3 years ago

I have also been coming across this bug, with IGV 2.8.9 freezing when receiving commands over a port. I came across this issue when trying to use the IGVNav app (https://github.com/griffithlab/igvnav), which provides a tool for manual variant review using IGV (developed in Python). When switching to the next variant (sending "goto 1:160000", receiving a response and then "sort base" to the port) IGV often freezes (about once every 2 or 3 calls). After it occurs, I have to restart both IGV and the IGVNav app.

I tried the suggestion of installing the snapshot release, but the problem still occurs. Using IGV 2.6.3 doesn't seem to have the problem however. Also, the log file doesn't show anything of note

jrobinso commented 3 years ago

@ConorMesser 2.8.x will not be fixed for this, as noted above. Also, this issue is specifically about ports, but it got diverted to running batch commands from startup. My comments about the snapshot are for the batch startup, I haven't yet looked into the port problem. Sorry for the confusion. Really there should be 2 issues. I should have some updates on both in the next few days.

jrobinso commented 3 years ago

@bbreton Let's move batch command line discussions to #876 , its really a separate issue (as opposed to port commands)

ConorMesser commented 3 years ago

@jrobinso yes, I got confused through the discussion. I will just work with 2.6 for now until 2.9 comes out. Let me know if you are able to eventually recreate the freezing issue.

jrobinso commented 3 years ago

@ConorMesser I was able to reproduce it, and I think fix it.

jrobinso commented 3 years ago

This should be fixed now in the snapshot build. Working towards a release (2.9) by the end of the month.