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 <[email protected]>
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 <[email protected]>
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.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
