[jira] [Commented] (FLINK-5642) queryable state: race condition with HeadListState

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FLINK-5642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15839722#comment-15839722 ] 

ASF GitHub Bot commented on FLINK-5642:
---------------------------------------

GitHub user NicoK opened a pull request:

    https://github.com/apache/flink/pull/3218

    [FLINK-5642][query] fix a race condition with HeadListState

    The idiom behind `AppendingState#get()` is to return a copy of the value behind or at least not to allow changes to the underlying state storage. However, the heap state backend returns the original `ArrayList` which is not thread-safe. In contrast to the operator/window evictor thread where only one accesses the state at a time, queryable state may access state any time in order not to slow down normal operation. Any structural changes to `ArrayList` are thus unsafe which is why this PR:
    * synchronizes access to structure-changing methods on that list (only for queryable state),
    * forbids `ArrayList#remove()` (only for queryable state) that is available through `Iterator#remove()` which is the only structural change the API offers on the `Iterable` that `HeapListState#get()` returns.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/NicoK/flink flink-5642

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/3218.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #3218
    
----
commit 542432d80ae79f773fe08d533222e71aee4c7da8
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-01-25T15:07:50Z

    [FLINK-5642][query] fix a race condition with HeadListState
    
    The idiom behind AppendingState#get() is to return a copy of the value behind
    or at least not to allow changes to the underlying state storage. However,
    the heap state backend returns the original list which is backed by an
    ArrayList which is not thread-safe. Aside from the operator/window evictor
    thread where only one accesses the state at a time, however, queryable state
    may access state anytime in order not to slow down normal operation. Any
    structural changes to ArrayList are thus unsafe and are hereby synchronized
    in case the state is queryable.

commit 641c2c9f46be4f4171a33923b1f1dc961464575f
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-01-25T15:04:15Z

    [FLINK-5642] fail when calling Iterator#remove() on queryable list state returned from HeapListState#get()
    
    The idiom behind AppendingState#get() is to return a copy of the value behind
    or at least not to allow changes to the underlying state storage. However,
    the heap state backend returns the original list and thus is prone to changes.
    The user cannot rely on changes to be reflected by the backing store but, if
    correctly used, e.g. by clearing the list and re-adding all elements afterwards,
    changes may still be ok.
    
    However, in conjunction with queryable state, any structural changes to the
    backing ArrayList lead to races (as may changes to the stored objects but we
    cannot forbid that for now). By forbidding ArrayList#remove(), we can at least
    forbid Iterator#remove() which is the only structural change the API offers
    on the Iterable that HeapListState#get() returns.

----


> queryable state: race condition with HeadListState
> --------------------------------------------------
>
>                 Key: FLINK-5642
>                 URL: https://issues.apache.org/jira/browse/FLINK-5642
>             Project: Flink
>          Issue Type: Bug
>          Components: Queryable State
>    Affects Versions: 1.2.0
>            Reporter: Nico Kruber
>            Assignee: Nico Kruber
>
> If queryable state accesses a HeapListState instance that is being modified during the value's serialisation, it may crash, e.g. with a NullPointerException during the serialisation or with an EOFException during de-serialisation.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)