Fixed concurrency issues in GradleDaemonExecutor

[mbien]

the internal GradleTask appears to cover s similar usecase as the
CompletableFuture class. The implementation however caused a three
state race condition:

• executor calls finish(int) before someone calls result(): this
  seems to be the intended usecase to create a fast-path which
  finishes the task early
• result() is called before finish(int): this will block on the
  wrapped task and finish has no effect. This behavior can be
  reproduced by attempting to delete a gradle project, the delete
  project dialog will stay open for almost a minute.
• third state is when createTask() is called too late: this will cause
  a NPE since the task is already started via setTask(), the
  executor would try to call finish on a task wrapper which is null.
  Can be easily reproduced by setting a breakpoint in the codepath
  which calls createTask() or the method itself
  example:

  java.lang.NullPointerException: Cannot invoke "org.netbeans.modules.gradle.execute.GradleDaemonExecutor$GradleTask.finish(int)" because "this.gradleTask" is null
  	at org.netbeans.modules.gradle.execute.GradleDaemonExecutor.run(GradleDaemonExecutor.java:263)
  	at org.netbeans.core.execution.RunClassThread.doRun(RunClassThread.java:132)
  	at org.netbeans.modules.openide.util.GlobalLookup.execute(GlobalLookup.java:45)
  	at org.openide.util.lookup.Lookups.executeWith(Lookups.java:287)
  [catch] at org.netbeans.core.execution.RunClassThread.run(RunClassThread.java:81)
  

fixes:

• block on CountDownLatch instead of delegate
• create wrapper right when the task starts
• the other commit fixes an issue where a start() is called from within the thread's constructor

this has also positive performance side effects: e.g project creation is also faster now

[mbien]

all green, removing all-tests label

[lkishalmi]

Great find!
Thank you!

[sdedic]

In this case, if called before finish, the result will appear to be 0 == success. Is that OK ? When searching for finish calls, I've found paths in GradleDaemonExecutor::run - the exception handlers, that do not call gradleTask.finish at all, so these all will reach this lambda and produce result 0 ?

[mbien]

not sure if I understand the question correctly. If someone calls result() and nothing calls the fast-path finish(int) the delegate task will count down the latch once it completes and the blocking result() method will now unblock and return delegate.result().

the second case is that finish(int) is called before the delegate task completes, this would also unblock all waiting threads which wait on the latch, but this time it returns the result field without waiting for the delegate task.

the second case seems to be the intended purpose of the task wrapper to speed up the time-to-result. Git blame msg luckily mentions it too d7d4faf I wished this would be mentioned in code.

[mbien]

I've found paths in GradleDaemonExecutor::run - the exception handlers, that do not call gradleTask.finish at all, so these all will reach this lambda and produce result 0 ?

@sdedic this was the case before too, no?

            if (result != null) {
                return result;
            }
            return this.delegate.result();

this would have blocked on the delegate task and return the result. I am not 100% sure what you mean by 0 since the current behavior should do the same.

[mbien]

planning to merge this soon unless there are objections

[mbien]

no objections after another 5 days -> going to refresh this PR and then merge when green. Thanks for reviewing everyone!