On Fri, 10 Feb 2023 11:46:22 GMT, Prasanta Sadhukhan <psadhuk...@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
