ConcurrentModificationException inside getSecondaryResources #2567

Activity

1. metacosm commented on Nov 6, 2024

   

   metacosm

   on Nov 6, 2024

   Last edited by metacosm

   Collaborator

   More actions

   Unfortunately, baring a JDK bug, I don't see anything in the code that would cause this problem since these operations are read-only. Let us know if you manage to replicate the issue with some additional context, please.
2. matteriben commented on Nov 6, 2024

   

   matteriben

   on Nov 6, 2024

   Last edited by matteriben

   ContributorAuthor

   More actions

   I'm not sure I can replicate it easily. But I think I can easily demonstrate the underlying issue.

       @Test
       void testConcurrency() throws InterruptedException {
           AtomicBoolean done = new AtomicBoolean();
           while (!done.get()) {
               Phaser phaser = new Phaser(2);
               Map<Object, Object> map = new HashMap<>();
               for (int i = 0; i < 10; i++) {
                   map.put(i, i);
               }
               Stream<Object> stream = map.values().stream();
               Thread thread1 = new Thread(() -> {
                   phaser.arriveAndAwaitAdvance();
                   for (int i = 10; i < 20; i++) {
                       map.put(i, i);
                   }
               });
               Thread thread2 = new Thread(() -> {
                   phaser.arriveAndAwaitAdvance();
                   stream.collect(Collectors.toList());
               });
               thread2.setUncaughtExceptionHandler((t, e) -> {
                   if (e instanceof ConcurrentModificationException) {
                       e.printStackTrace(System.out);
                       done.set(true);
                   }
               });
               thread1.start();
               thread2.start();
               thread1.join();
               thread2.join();
           }
       }
   

   java.util.ConcurrentModificationException
   	at java.base/java.util.HashMap$ValueSpliterator.forEachRemaining(HashMap.java:1792)
   	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
   	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
   	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
   	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
   	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
   

   👀React with 👀1Reacted by Chris Laprun
3. metacosm commented on Nov 7, 2024

   

   metacosm

   on Nov 7, 2024

   Collaborator

   More actions

   OK so the issue would happen when your ResourceDiscriminator would be called while other event sources are still being initialized? If you are initializing your event sources manually, you could try to set the event source's priority to EventSourceStartPriority.RESOURCE_STATE_LOADER to see if this addresses the issue (you can see an example of this being used at https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/externalstate/ExternalStateReconciler.java#L107).
   If you're not setting up sources manually, could you detail how they are set up, please? Maybe we could (should?) surface that priority setting?
4. matteriben commented on Nov 7, 2024

   

   matteriben

   on Nov 7, 2024

   ContributorAuthor

   More actions

   This reconciler is "managed", in the sense that dependent resources are added through annotations. No event sources for this reconciler are initialized manually.

   This reconciler does however have six dependent resources with activation conditions, including the dependent resource whose ResourceDiscriminator was called when this exception was thrown. I could be wrong, but I suspect this may be related to the activation conditions.

   AbstractWorkflowExecutor::registerOrDeregisterEventSourceBasedOnActivation

   EventSourceManager::dynamicallyRegisterEventSource

   EventSourceManager::registerEventSource

   EventSources::add

   👍React with 👍2Reacted by Attila Mészáros and Chris Laprun
5. csviri commented on Nov 7, 2024

   

   csviri

   on Nov 7, 2024

   Collaborator

   More actions

   Yes, event sources are added dynamically with activation condition, this will be the issue
6. csviri

   self-assigned this

   on Nov 12, 2024
7. csviri

   linked a pull request that will close this issuefix: concurrent modification when getting event sources (v5) #2572on Nov 12, 2024
8. csviri

   linked a pull request that will close this issuefix: concurrent modification when getting event sources (v4) #2573on Nov 12, 2024
9. csviri commented on Nov 12, 2024

   

   csviri

   on Nov 12, 2024

   Last edited by csviri

   Collaborator

   More actions

   Added fix for both main and v5, pls take a look if it makes sense.

   👍React with 👍1Reacted by Chris Laprun
10. csviri

    closed this as completedin #2573on Nov 13, 2024