WFCORE-7623: signal if input is idle#6860
Conversation
| private Process cliProcess; | ||
| private volatile StringBuffer cliOutputBuffer = new StringBuffer(); | ||
| private BufferedWriter bufferedWriter = null; | ||
| private boolean idleInput = false; |
There was a problem hiding this comment.
This variable must be volatile, otherwise there will not be visibility guarantees between the three threads that modify/read it
| public String getCurrentPrompt(){ | ||
| if(cliOutputBuffer.toString().contains("\n")) { | ||
| return cliOutputBuffer.toString().substring(cliOutputBuffer.toString().lastIndexOf("\n")+1); | ||
| return cliOutputBuffer.substring(cliOutputBuffer.toString().lastIndexOf("\n")+1); |
There was a problem hiding this comment.
Please always apply the standard code formatting, otherwise in the future applying the format will generate changes
| if(cliOutputBuffer.toString().contains("\n")) { | ||
| return cliOutputBuffer.toString().substring(cliOutputBuffer.toString().lastIndexOf("\n")+1); | ||
| return cliOutputBuffer.substring(cliOutputBuffer.toString().lastIndexOf("\n")+1); |
There was a problem hiding this comment.
What hapends if after reading"cliOutputBuffer.toString().contains("\n")" the cliOutputBuffer changes in between before executing the return?
There was a problem hiding this comment.
The method is called after we made sure the buffer shouldn't be changing. But even if it changed it should still be returning the last line of the output, worse case lastIndexOf("\n")+1 is 0.
| if (!br.ready()) { | ||
| idleInput = idleCounter <= 1; | ||
| idleCounter++; | ||
| } else { | ||
| idleInput = false; | ||
| } |
There was a problem hiding this comment.
I do not get the behaviour of this logic. Could you explain it here? , why "idleCounter <= 1" and why you do it if the buffer is not ready?
There was a problem hiding this comment.
I understand the process will echo the user input, and here what you want to do is to signal when this echo has not finished yet. Also, I understand that the server output is processed as an stream, and I guess this is the main issue.
Correct me if I am wrong, each echo will be following by a new line, and in that moment the output of the command result will start, probably it will contain several lines.
I do not get how this will help with this:
which seems to be the main issue since outputHasPrompt could be invoked after the input was echoed by the server but the full command output is not fully processed yet.
Would not be easier to use a producer consumer pattern on the threads that reads the std out / err and produce line by line?, consumers will wait until a line is ready, consumers will not read lines in the middle of processing. In general, looks like that sharing the cliOutputBuffer between the CliResultsReader threads and the main thread is quite problematic
2 participants
Issue: WFCORE-7623
Follow-up to #6849
Since the wrapper is using blocking IO we can only guess if the input will be idle, but as far as I can see we can reliably tell if we're in the "waiting for user input" state, and only check the prompt at that point. This should prevent the tests from failing with a partially printed output, assuming whatever is happening on Windows is simply a timing issue.