[DATACMNS-1758] Fix ConcurrencyModificationException in DefaultEntityCallbacks #453
Conversation
mhyeon-lee
force-pushed
the
datacmns-1758-fix-concurrentmodificationexception
branch
from
a874f4c
to
bc037bc
Compare
| @@ -394,7 +394,7 @@ private BeanFactory getRequiredBeanFactory() { | |||
| } | |||
| } | |||
|
|
|||
| return cachedEntityCallbacks; | |||
| return new ArrayList<>(cachedEntityCallbacks); | |||
There was a problem hiding this comment.
I fixed it again.
I think it's safer to handle it where it returns the result.
| } | ||
|
|
||
| return new ArrayList<>(cachedEntityCallbacks); |
There was a problem hiding this comment.
new ArrayList also has a problem.
If cachedEntityCallbacks.clear() is executed on another thread before this line is executed, an emptyList is returned.
I think it is easier to change the new cache list. (Give up the final)
Thread 1 | Thread2
----------------------
clear() |
| new ArrayList(cachedEntityCallbacks);
addAll() |
There was a problem hiding this comment.
In that case, it's probably better to roll back the entire optimization to a state where it has been before. That way, we omit all concurrency issues in the first place.
There was a problem hiding this comment.
@mp911de
This is not an optimization, but a fix to solve the concurrency issue.
getEntityCallbacks method returns cachedEntityCallbacks and DefaultEntityCallbacks use it with iterator
f9839c8#diff-f9f021397523461fe28f39d98bb39f39L83
However, cachedEntityCallbacks can be modified here. (clear, addAll)
This throws ConcurrentModificationException.
Exception:java.util.ConcurrentModificationException: null
at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1042)
at java.base/java.util.ArrayList$Itr.next(ArrayList.java:996)
at org.springframework.data.mapping.callback.DefaultEntityCallbacks.callback(DefaultEntityCallbacks.java:83)
at org.springframework.data.jdbc.core.JdbcAggregateTemplate.triggerBeforeSave(JdbcAggregateTemplate.java:414)
at org.springframework.data.jdbc.core.JdbcAggregateTemplate.store(JdbcAggregateTemplate.java:335)
at org.springframework.data.jdbc.core.JdbcAggregateTemplate.update(JdbcAggregateTemplate.java:183)
Addresses concurrency issues by replacing the entire cachedEntityCallbacks without changing them when there are changes.
There was a problem hiding this comment.
@mp911de
I understood your words. Thank you.
Should I close this PR?
There was a problem hiding this comment.
Yes, it makes sense that we fix our own issue.
resolves: DATACMNS-1758