Duplicate deletion in ITC

[abelhegedus]

Short version: it is possible to reach a state where the ITC algorithm sends duplicate removal messages to the Rete engine, causing an exception.

Long version: When two specific (and quite complex) queries are both matched on a model, removing elements from the model, that changes the result sets of these queries, results in erroneous messages from the ITC.

I have queries and models and a test code that consistently reproduces the error:

• model: https://gist.github.com/4067344#file_descent_2.ecore
• queries: https://gist.github.com/4067344#file_test.eiq
• test code: https://gist.github.com/4067344#file_strange_deletion_test.xtend

The test loads an Ecore model and a pattern model with some queries, including several transitive closure ones. After initial matching, some of the EClasses are deleted. If the "canTransitivelyContain" and the "reference" queries are both matched, the error occurs. If only one or neither is there, it doesn't.

We have instrumented the TransitiveClosureNode to see what kind of messages are used at the initialization and later in the model modification case.

This output is expressed in the following: https://gist.github.com/4067344#file_transitive_closure_node

@bergmanngabor even created a small drawing of the setup: https://www.dropbox.com/s/8heupfugcmtqlwu/WP_000168.jpg

The deletion of the "OTCPM->RCP" (abbreviations for the classes in the test Ecore model) edge is the catalyst of the problems.

[bergmanngabor]

The deletion of the "OTCPM->RCP" (abbreviations for the classes in the test Ecore model) edge is the catalyst of the problems.

Yes, the TC algorithm will report the loss of transitive reachability D--..->RC and G-..->RC, even though they still have an alternate derivation due to base edge D->RC. The exception will only be triggered, of course, when the loss of the same pairs will be reported again by the TC node; but this "OTCPM->RCP" deletion is when the TC actually goes wrong.

[szabta89]

@abelhegedus Can you upload to code as a plug-in in order for me to test it?

[abelhegedus]

The attached code is a completely working JUnit Plug-in Test that uses absolute file paths to load the test inputs that I also uploaded.

However, here is the project I used: https://www.dropbox.com/s/q8lx758hobo7btx/test.multi.genpackage.zip

[szabta89]

@abelhegedus What can I do to make the test more deterministic? Redoing the same sequence of steps from your trace does not result an NPE for me, however, I do have encountered NPE when running the test of yours.

[szabta89]

@abelhegedus Please take a look at the current state of the code. I think the previous commit solved the issue.

[abelhegedus]

Yes, the issue is gone. Thank you!

[abelhegedus]

Reopening, as cherry picking is needed, I think.

[szabta89]

@abelhegedus Cherry pick is done.

[ujhelyiz]

@szabta89 Please, write comments in your source code.

Please, also confirm that the test case deterministically reproduces the original issue, and your fix resolves it.

[szabta89]

@ujhelyiz The test case reproduces the problem deterministically and the commit solved it. There was a problem with the initialization of the Counting algorithm (the IncSCC depends on it). I'm gonna pay more attention on the comments.