[GitHub] [incubator-daffodil] jadams-tresys commented on a change in pull request #375: Fixed issues with resetting variables

[GitBox <gi...@apache.org>]

jadams-tresys commented on a change in pull request #375:
URL: https://github.com/apache/incubator-daffodil/pull/375#discussion_r419030815



##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PState.scala
##########
@@ -220,6 +221,7 @@ final class PState private (
       if (variable.isDefined)
         variable.get.reset
     }}
+    changedVariablesStack.top.clear

Review comment:
       I have now added a comment to this. The reason why it needs to be cleared is that when parsing a choice (or potentially an unordered sequence) there can be multiple calls to PState.reset() within the same push/popPointOfUncertainty. Which means that the list on changedVariablesStack.top will continue to accumulate changes if the list is not cleared. For example, choice branch 1 reads a variable but ultimately fails calling PState.reset(), which will reset the read on the variable, but then the choice enters branch 2 without popping the POU stack. Branch 2 then sets the variable but ultimately fails too and when PState.reset() gets called it will reset the setVariable call and then try to reset the readVariable call from branch 1 since it hasn't been cleared and we are still in the same POU.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org