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). PR: https://git.openjdk.java.net/jfx/pull/17