RFR: 8081474: SwingWorker calls 'done' before the 'doInBackground' is finished [v14]

[Alexey Ivanov]

On Wed, 8 Feb 2023 09:46:21 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> SwingWorker done() method [spec ](https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/javax/swing/SwingWorker.java#L452) says "Executed on the Event Dispatch Thread after the doInBackground method is finished"
>> but there's no mechanism in place to honor that claim.
>> The [spec](https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/javax/swing/SwingWorker.java#L289)
>> also says the state should be DONE after doInBackground() returns which is also not done.
>> 
>> Modified the code to honour the specification.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Test updated

I admit I got confused … by now.

It's been a long discussion.

> My take is current code fix and spec clarification is done to make code/spec wording consistent with **existing** spec whereas this change is to **enhance** the code/spec which I think we should keep it separate from this fix, as this will cause JCK test to be excluded and will cause JCK to update the test and I am not sure how that will pan out, so I suggest to keep the change to minimal to the extent only which is needed for this fix..

Yet the spec change is unnecessary if you change the order of calls as [discussed above](https://github.com/openjdk/jdk/pull/11940#discussion_r1095103200).

Referring to [the same thread](https://github.com/openjdk/jdk/pull/11940#discussion_r1095741184),

> > > It seems the order of sequence is listener->State.STARTED->doInBackground->listener->DONE->done
> > 
> > Yes, but the different order is specified: listener(STARTED) -> doInBackground -> done -> listener(DONE).
> >
> But then it will violate
> 
> https://github.com/openjdk/jdk/blob/810c8a271b4524ae776e2306ef699e04a7d145a2/src/java.desktop/share/classes/javax/swing/SwingWorker.java#L288-L293

No, it doesn't violate: the state transitions to `DONE` *after method is finished*.

And I realised [I had addressed it already](https://github.com/openjdk/jdk/pull/11940#discussion_r1095781185).

[Sergey is right](https://github.com/openjdk/jdk/pull/11940#discussion_r1099522374) there is *no contradiction*. It is how it is specified.

“The spec for `DONE` state does not specify when the transition occurs.” It says _after_, which is satisfied in either case: 1) `done` method is called, state transitions to `DONE`; or 2) state transitions to `DONE`, then `done` method is called.

The first case is how it was implemented before this fix. The second case is how it is implemented at this very moment in the this fix.


If you change the order of these two calls

https://github.com/openjdk/jdk/blob/76b02ea3a8add954705d6df7f91f5896d141b02d/src/java.desktop/share/classes/javax/swing/SwingWorker.java#L307-L308

you'll bring back the code to case 1 which aligns with the current specification.

If you do it, Sergey's concern [in this comment](https://github.com/openjdk/jdk/pull/11940#discussion_r1099522374) *will be resolved*. (For convenience: “Above we discussed that it is possible to see a difference if the listener will do some work on EDT, after the fix the listener will not be called last, is that safe to assume it does not break something?”)

Even though I don't see it as a problem, it is safer to preserve the specified order. Do you agree?

-------------

PR: https://git.openjdk.org/jdk/pull/11940