search

rwth-acis / moodle-data-proxy

3 stars 1 forks source link

[BUG] Time of event checking and concurrent modification #22

Open BorisJov opened 3 years ago

BorisJov commented 3 years ago
  1. Summary - While testing on another issue, I came across a java.util.ConcurrentModificationException. Upon further inspection I noticed a timestamp validity check that seems problematic.
  2. Bug Details
    1. What? - We seem to be checking if updates happened before the proxy's local time as a sort of validity check, if the updates happened after, we remove them from the array that is being iterated over, which causes the ConcurentModificationException (textbook case).
    2. Where? - It happens in this loop.
    3. When?/How often? - It seems to occur when rapid successions of events are generated. This also obviously happens when the moodle services local time is later than the proxy's.
    4. How?/Current state - The generated exception was such: java.util.ConcurrentModificationException at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:909) at java.util.ArrayList$Itr.next(ArrayList.java:859) at i5.las2peer.services.moodleDataProxyService.MoodleDataProxyService$DataStreamThread.run(MoodleDataProxyService.java:197) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) at java.lang.Thread.run(Thread.java:748)
  3. Fixed state - The proxy probably shouldn't be making this validity check at all, it's own local time should be irrelevant to the system, only moodle's time should be the relevant one (and the LRS's). This check might have been implemented because of duplicate statements being generated, therefore the check will be removed with caution
BorisJov commented 3 years ago

Perhaps looking at the time delta is a good idead. Maybe extract the difference from the HTTP header?