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

[Prasanta Sadhukhan]

On Fri, 10 Feb 2023 11:46:22 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 two additional commits since the last revision:
> 
>  - Fix and test updated
>  - Fix and 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?

Yes, I see..I have updated the fix and test.

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

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