On Mon, 30 Jan 2023 08:34:59 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org>
wrote:
>> src/java.desktop/share/classes/javax/swing/SwingWorker.java line 762:
>>
>>> 760: } catch (InterruptedException e) {}
>>> 761: } while (state != StateValue.DONE);
>>> 762: }
>>
>> In the previous iteration, using `sleep` for waiting was *the concern*,
>> you're still using `sleep`.
>>
>> This is not going to work because it makes `cancel` wait until `DONE` state
>> is reached, which is not what one would expect, especially taking into
>> account that `cancel` is likely called from EDT to cancel the background
>> operation and blocking EDT is not acceptable.
>
> I removed sleep from EDT case and not blocking EDT and guess sleep is now
> being done for non-EDT case...
> If it is to be called from EDT then it should go to "if" block and not to
> "else" which is what I based my fix on..
>
> Anyway, I appreciate your fix and will see to it..
Yet having `sleep` is an indication of busy wait. You could've used
`CountDownLatch` in conjunction with `setState` to avoid `sleep` at all.
> I removed sleep from EDT case and not blocking EDT and guess sleep is now
> being done for non-EDT case...
Ah, right. But then if `cancel` is called on EDT, `done` is invoked before
`doInBackground` completes, isn't it?
If `cancel` is called from another thread, the main thread as in the test, it
doesn't return until `doInBackground` completes. With your test case and fix,
`cancel` blocks for 5 seconds.
Either way, the behaviour does not look right.
-------------
PR: https://git.openjdk.org/jdk/pull/11940
