Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add validation for a modifying thread in VirtualMap #10583

Closed
imalygin opened this issue Dec 19, 2023 · 0 comments · Fixed by #10584
Closed

Add validation for a modifying thread in VirtualMap #10583

imalygin opened this issue Dec 19, 2023 · 0 comments · Fixed by #10584
Assignees
Labels
Improvement Code changes driven by non business requirements.
Milestone

Comments

@imalygin
Copy link
Member

Problem

This is issue is a follow up for recent ISS investigation. The following exception happened

java.lang.NullPointerException: null
at java.util.Objects.requireNonNull(Objects.java:208) ~[?:?]
at com.swirlds.virtualmap.internal.merkle.VirtualRootNode.add(VirtualRootNode.java:1582) ~[swirlds-virtualmap-0.43.4.jar:?]
at com.swirlds.virtualmap.internal.merkle.VirtualRootNode.put(VirtualRootNode.java:833) ~[swirlds-virtualmap-0.43.4.jar:?]
at com.swirlds.virtualmap.VirtualMap.put(VirtualMap.java:463) ~[swirlds-virtualmap-0.43.4.jar:?]
at com.hedera.node.app.service.mono.state.adapters.VirtualMapLike$1.put(VirtualMapLike.java:99) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.state.migration.TokenRelStorageAdapter.put(TokenRelStorageAdapter.java:67) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.state.expiry.TokenRelsListMutation.put(TokenRelsListMutation.java:55) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.state.expiry.TokenRelsListMutation.put(TokenRelsListMutation.java:27) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.utils.MapValueListUtils.internalAddFirstInPlaceForMapValueList(MapValueListUtils.java:198) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.utils.MapValueListUtils.insertInPlaceAtMapValueListHead(MapValueListUtils.java:48) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.ledger.interceptors.TokenRelsLinkManager.updateLinks(TokenRelsLinkManager.java:81) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.ledger.interceptors.LinkAwareTokenRelsCommitInterceptor.lambda$preview$2(LinkAwareTokenRelsCommitInterceptor.java:120) ~[app-service-mono-0.43.4.jar:?]
at java.lang.Iterable.forEach(Iterable.java:75) ~[?:?]
at com.hedera.node.app.service.mono.ledger.interceptors.LinkAwareTokenRelsCommitInterceptor.preview(LinkAwareTokenRelsCommitInterceptor.java:119) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.ledger.TransactionalLedger.commit(TransactionalLedger.java:240) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.ledger.HederaLedger.commit(HederaLedger.java:206) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.state.logic.ServicesTxnManager.attemptCommit(ServicesTxnManager.java:149) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.state.logic.ServicesTxnManager.process(ServicesTxnManager.java:132) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.state.logic.StandardProcessLogic.doProcess(StandardProcessLogic.java:178) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.state.logic.StandardProcessLogic.incorporate(StandardProcessLogic.java:137) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.state.logic.StandardProcessLogic.incorporateConsensusTxn(StandardProcessLogic.java:108) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.txns.ProcessLogic.lambda$incorporateConsensus$0(ProcessLogic.java:40) ~[app-service-mono-0.43.4.jar:?]
at com.swirlds.common.system.Round.forEachEventTransaction(Round.java:95) ~[swirlds-common-0.43.4.jar:?]
at com.hedera.node.app.service.mono.txns.ProcessLogic.incorporateConsensus(ProcessLogic.java:39) ~[app-service-mono-0.43.4.jar:?]
at com.hedera.node.app.service.mono.ServicesState.handleConsensusRound(ServicesState.java:302) ~[app-service-mono-0.43.4.jar:?]
at com.swirlds.platform.state.TransactionHandler.handleRound(TransactionHandler.java:84) ~[swirlds-platform-core-0.43.4.jar:?]
at com.swirlds.platform.state.SwirldStateManagerImpl.handleConsensusRound(SwirldStateManagerImpl.java:210) ~[swirlds-platform-core-0.43.4.jar:?]
at com.swirlds.platform.eventhandling.ConsensusRoundHandler.applyConsensusRoundToState(ConsensusRoundHandler.java:414) ~[swirlds-platform-core-0.43.4.jar:?]
at com.swirlds.common.threading.framework.internal.QueueThreadImpl.doWork(QueueThreadImpl.java:253) ~[swirlds-common-0.43.4.jar:?]
at com.swirlds.common.threading.framework.internal.StoppableThreadImpl.doWork(StoppableThreadImpl.java:614) ~[swirlds-common-0.43.4.jar:?]
at com.swirlds.common.threading.framework.internal.StoppableThreadImpl.run(StoppableThreadImpl.java:215) ~[swirlds-common-0.43.4.jar:?]
at java.lang.Thread.run(Thread.java:833) 

@OlegMazurov conducted thorough investigation and figured out that the best theory explaining this that VirtualMap may be used improperly. Specifically, it may happen if there are two concurrent modifications of it, and this data structure is designed so that it does not guarantee correct behavior in this case. That is, there should be only one modifying thread at a time, otherwise it may result in the above-mentioned exception.

Solution

To catch situations like this VirtualRootNode modifying methods including but not limited to put and add must have asserts verifying that there is no other thread at the moment executing any modifying method on this VirtualMap

Alternatives

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Code changes driven by non business requirements.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant