Devs: Java -> streams -> forEach can broke the game (lost priority bug)?

[JayDi85]

Example code from Whirlwind Denial:

game.getStack()
                .stream()
                .filter(Objects::nonNull)
                .forEachOrdered(stackObject -> {
                    Cost cost = new GenericManaCost(4);
                    if (cost.canPay(source, source.getSourceId(), source.getControllerId(), game)
                            && player.chooseUse(outcome, "Pay {4} to prevent " + stackObject.getIdName() + " from being countered?", source, game)
                            && cost.pay(source, game, source.getSourceId(), source.getControllerId(), false)) {
                        return;
                    }
                    stackObject.counter(source.getSourceId(), game);
                });
        return true;
    }

It uses a direct game objects list like Stack. If something can change it inside then it will be broken by ConcurrentModificationException (example with stack processing - if something add or remove from stack). It can be modified in the same code or by ApplyEffect, ProcessActions, game events and other “hidden” logic.

Recommends:

• if you need to do a game state changes to the same objects collection then collect it to the own list and make changes from it;
• some standard methods like getActivePermanents already returns a new list — so it’s safe for game state modifications.

[jeffwadsworth]

Was this ever validated or not? It is a pretty important issue nowadays.

[JayDi85]

I dodn't research it yet. Potentially broken code -- with native lock/wait calls. If game thread make object's lock (lock or synchronized call), but later it return to that locked object from stream's code -- that can freeze the game in infinite wait. Locks uses for server's thread and user's feedbacks. Added in 2024: See starting message with examples

In current server's logs I can find rare concurrent access/modification exceptions -- but it maybe something else. Example:

INFO  2020-01-23 07:01:54,832 GAME started f81229c0-e78c-4195-88e0-5c26452d682e [Standard] BruceWayne - computer         =>[CALL main-2993] TableController.startGame
Exception in thread "CALL main-3000" java.util.ConcurrentModificationException
        at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:909)
        at java.util.ArrayList$Itr.next(ArrayList.java:859)
        at mage.abilities.effects.ContinuousEffects.preventedByRuleModification(ContinuousEffects.java:735)
        at mage.game.GameState.replaceEvent(GameState.java:716)
        at mage.game.GameState.replaceEvent(GameState.java:712)
        at mage.game.GameImpl.replaceEvent(GameImpl.java:2639)
        at mage.players.PlayerImpl.canLose(PlayerImpl.java:2499)
        at mage.players.PlayerImpl.lost(PlayerImpl.java:2473)
        at mage.players.PlayerImpl.concede(PlayerImpl.java:2392)
        at mage.game.GameImpl.concede(GameImpl.java:1196)
        at mage.server.game.GameController.sendPlayerAction(GameController.java:529)
        at mage.server.game.GameManager.sendPlayerAction(GameManager.java:111)
        at mage.server.MageServerImpl.lambda$null$37(MageServerImpl.java:814)
        at java.util.Optional.ifPresent(Optional.java:159)
        at mage.server.MageServerImpl.lambda$sendPlayerAction$38(MageServerImpl.java:812)
        at mage.server.MageServerImpl.lambda$execute$67(MageServerImpl.java:1161)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)

P.S. It's only theory. added in 2024:that’s error example related to wrong call thread, see #11460

[JayDi85]

As a preventive measure: try not use any user intercation code (pay/choose/etc) from streams (e.g. from forEach).

[JayDi85]

[[Whirlwind Denial]] can cause ConcurrentModificationException error. Possible use case: stack contains multiple abilities and try to counter some of it.

java.util.ConcurrentModificationException
	at java.util.ArrayDeque$DeqSpliterator.forEachRemaining(ArrayDeque.java:957)
    	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
    	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
    	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
    	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
    	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    	at java.util.stream.ReferencePipeline.forEachOrdered(ReferencePipeline.java:423)
    	at mage.cards.w.WhirlwindDenialEffect.apply(WhirlwindDenial.java:61)
    	at mage.abilities.AbilityImpl.resolveMode(AbilityImpl.java:198)
    	at mage.abilities.AbilityImpl.resolve(AbilityImpl.java:185)
    	at mage.game.stack.Spell.resolve(Spell.java:244)
    	at mage.game.GameImpl.resolve(GameImpl.java:1659)
    	at mage.game.GameImpl.playPriority(GameImpl.java:1585)
    	at mage.game.turn.Step.priority(Step.java:61)
    	at mage.game.turn.Phase.playStep(Phase.java:184)
    	at mage.game.turn.Phase.play(Phase.java:89)
    	at mage.game.turn.Turn.play(Turn.java:125)
    	at mage.game.GameImpl.playTurn(GameImpl.java:1058)
    	at mage.game.GameImpl.play(GameImpl.java:968)
    	at mage.game.GameImpl.start(GameImpl.java:944)
    	at mage.server.game.GameWorker.call(GameWorker.java:34)
    	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)

[github-actions]

Whirlwind Denial - (Gatherer) (Scryfall) (EDHREC)

{2}{U}
Instant
For each spell and ability your opponents control, counter it unless its controller pays {4}.