On Wed, 4 Dec 2019 15:29:50 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
> On Wed, 4 Dec 2019 14:50:21 GMT, Robert Lichtenberger <rlich...@openjdk.org> > wrote: > >> On Tue, 3 Dec 2019 05:08:49 GMT, Ambarish Rapte <ara...@openjdk.org> wrote: >> >>> On Mon, 21 Oct 2019 10:19:04 GMT, Robert Lichtenberger >>> <rlich...@openjdk.org> wrote: >>> >>>> By using the collection itself as synchronization lock we achieve >>>> behaviour that matches java.util.Collections classes. >>>> >>>> I've create test cases that fail with the current way of synchronizing on >>>> a separate object. >>>> >>>> I've removed unused constructors. >>>> >>>> ---------------- >>>> >>>> Commits: >>>> - 7e80839f: 8232524: SynchronizedObservableMap cannot be be protected for >>>> copying/iterating >>>> - 8ecf3545: JDK-8232524 fixed. >>>> >>>> Changes: https://git.openjdk.java.net/jfx/pull/17/files >>>> Webrev: https://webrevs.openjdk.java.net/jfx/17/webrev.00 >>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8232524 >>>> Stats: 120 lines in 2 files changed: 95 ins; 17 del; 8 mod >>>> Patch: https://git.openjdk.java.net/jfx/pull/17.diff >>>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/17/head:pull/17 >>> >>> The change looks good to me, added a comment for a small change in test. >>> >>> modules/javafx.base/src/test/java/test/javafx/collections/FXCollectionsTest.java >>> line 730: >>> >>>> 729: } catch (ConcurrentModificationException e) { >>>> 730: fail("ConcurrentModificationException should not be >>>> thrown"); >>>> 731: } >>> >>> The thread should be terminated here too, please add `thread.terminate();` >>> before `fail()` >>> >>> ---------------- >>> >>> Changes requested by arapte (Reviewer). >> >> You're right. I just pushed the fix. > > Note that this is still pending a second review from @arapte @effad you can now integrate this whenever you are ready. I will sponsor it. PR: https://git.openjdk.java.net/jfx/pull/17
