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

Reply via email to