[FLINK-8226] [cep] Dangling reference generated after NFA clean up timed out SharedBufferEntry #5141
Conversation
…med out SharedBufferEntry
|
@dawidwys Could you help to take a look at this PR? This is a bug fix and the issue can be easily reproduced with the test case included in the PR. Thanks a lot. |
|
|
||
| private transient Map<K, SharedBufferPage<K, V>> pages; | ||
|
|
||
| private final transient List<SharedBufferEntry<K, V>> prunedEntries; |
There was a problem hiding this comment.
Is it necessary for it to be a field? Isn't a local variable sufficient?
There was a problem hiding this comment.
It is for object reuse. What's your thought?
There was a problem hiding this comment.
Not sure about the potential advantages. See e.g. this answer on SO: https://stackoverflow.com/a/18371813/4250114 . Plus the local variable is easier to reason about.
Personally I am for the local variable, but if you feel this version is better, will merge it as it is.
There was a problem hiding this comment.
Make sense, I will change it to local variable.
There was a problem hiding this comment.
Updated the PR. Thanks a lot for the review. :)
| for (Map.Entry<K, SharedBufferPage<K, V>> entry : pages.entrySet()) { | ||
| entry.getValue().removeEdges(prunedEntries); | ||
| } | ||
| prunedEntries.clear(); |
There was a problem hiding this comment.
As prunedEntries is local now, you don't need to clear it any longer.
…med out SharedBufferEntry This closes #5141
…med out SharedBufferEntry This closes apache#5141
3 participants
What is the purpose of the change
This pull request fix the issue that dangling reference generated after NFA clean up timed out SharedBufferEntry. Exception will be thrown when serializing NFA.
Verifying this change
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (no)Documentation