fixes for #544 and #545 #549
Conversation
| @@ -29,6 +31,7 @@ public void evaluate() throws Throwable { | |||
|
|
|||
| private StatementThread evaluateStatement() throws InterruptedException { | |||
| StatementThread thread = new StatementThread(fOriginalStatement); | |||
| thread.setDaemon(true); | |||
There was a problem hiding this comment.
Can you add a comment about why this is necessary?
There was a problem hiding this comment.
@dsaff
Sure, i can put a comment to the code.
For us here this would explain the problem more simply than FailOnTimeout.
The application does not exit unless setDaemon(true) is uncommented.
Thread t = new Thread() {
public void run() {
while (true) {
try {
System.out.println("run");
Thread.sleep(1000);
} catch (InterruptedException e) {
System.out.println(e.getLocalizedMessage());
}
}
}
};
/// t.setDaemon(true);
t.start();
t.join(1100);
t.interrupt();
There was a problem hiding this comment.
I was just thinking something like "Let the process complete even if this thread is not finished."
There was a problem hiding this comment.
Can you put the comment on its own line? (Style preference.) Thanks.
There was a problem hiding this comment.
Again, can you put the comment on its own line?
There was a problem hiding this comment.
Again, can you put the comment on its own line?
There was a problem hiding this comment.
Yes, i will.
I missed that, sorry.
There was a problem hiding this comment.
Sorry if I've been confusing. What I mean is that I find inline comments like this (especially without spaces) hard to read. So I'm recommending:
//Let the process/application complete after timeout expired.
thread.setDaemon(true);
|
changes:
|
| * When declaring 'volatile' variables, the CPU is forced | ||
| * to reconcile the values stored in registers and thread's stack | ||
| * by cache coherence at every write-read operation on these variables | ||
| * ; Otherwise the CPU and VM may reconcile the memories as it wants |
There was a problem hiding this comment.
Odd to put semicolon at the beginning of a line.
There was a problem hiding this comment.
I will fixt it.
It will stay at end of previous line.
|
@dsaff |
|
This looks very simple pull. Nothing special. Setting Some final and volatile modifier as well as nice thread safety -to make sure we made all for 100% visibility. |
| * Besides visibility, the volatile variables have also other guarantees: | ||
| * atomicity and ordering. | ||
| * */ | ||
| private volatile boolean fFinished; |
There was a problem hiding this comment.
We don't need a lesson on what volatile means. That's easy to Google. We need to indicate why these fields need to be volatile, and the others don't.
|
@Tibor17, I've made some comments in the code. Can you take a look? |
|
I answered all pending questions briefly.
|
| Random rnd = new Random(); | ||
| byte[] data = new byte[1024]; | ||
| File tmp = tmpFile.newFile(); | ||
| tmp.deleteOnExit(); |
|
changes:
|
|
Thanks! |
Simple fixes for #544 and #545.
Changes: