RFR: 8232524: SynchronizedObservableMap cannot be be protected for copying/iterating
Kevin Rushforth
kcr at openjdk.java.net
Wed Dec 4 23:19:29 UTC 2019
On Wed, 4 Dec 2019 15:29:50 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
> On Wed, 4 Dec 2019 14:50:21 GMT, Robert Lichtenberger <rlichten at openjdk.org> wrote:
>
>> On Tue, 3 Dec 2019 05:08:49 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:
>>
>>> On Mon, 21 Oct 2019 10:19:04 GMT, Robert Lichtenberger <rlichten at 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
More information about the openjfx-dev
mailing list