Issue 154 in soar: inconsistent crash on epmem query

[so...@googlecode.com]

Status: New
Owner: justinn...@gmail.com
CC: maz...@gmail.com, alex.nic...@soartech.com
Labels: Type-Defect Priority-High

New issue 154 by marin...@gmail.com: inconsistent crash on epmem query
http://code.google.com/p/soar/issues/detail?id=154

I have a complex agent that uses epmem. On windows, sometimes when epmem is
queried, Soar crashes. We've also run this code on linux and do not see the
crash there, although it's possible that it's just more rare. I can't
publicly provide specific code to reproduce this problem, but I can give
details of where the crash is occurring in the kernel. (On windows it
usually crashes, but not always.)

We are using a build off the trunk, r13955. Windows 7 pro w/Visual Studio
2013 Express and Linux Mint 13 Maya.

The nature of the episodes is that there is a multi-valued attribute. The
query contains one of the possible values in the multi-valued attribute. (I
don't know if this is relevant, but it could be.)

When the crash occurs, windows gives this error message:

According to Visual Studio, the line of code causing the crash is line 4170
in episodic_memory.cpp:

for (epmem_node_pair_set::iterator node_iter = child_lit->matches.begin();
node_iter != child_lit->matches.end(); node_iter++) {

The issue may be that somehow node_iter is already at the end and then it's
being incremented. This could be the case if the matches collection is
being modified inside the loop (which appears to be a recursive call to the
function that contains this loop). There appear to be several checks in the
function to make sure matches is not 0 length before doing some operations,
so whoever wrote this was aware that this could happen, but it appears that
at least one codepath is still making it through somehow.

While I can't share the code here, if it would help, someone could come to
SoarTech and we could demonstrate the issue there.

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[so...@googlecode.com]

Comment #1 on issue 154 by marin...@gmail.com: inconsistent crash on epmem
query
http://code.google.com/p/soar/issues/detail?id=154

Sorry, I left out the actual error message. I've attached a screenshot.

Attachments:
error.png 56.9 KB

[so...@googlecode.com]

Comment #2 on issue 154 by adam.syp...@soartech.com: inconsistent crash on
epmem query
http://code.google.com/p/soar/issues/detail?id=154

We think we have a fix for this issue. Currently, the three lines beginning
in episodic_memory.cpp:4170 are:

for (epmem_node_pair_set::iterator node_iter = child_lit->matches.begin();
node_iter != child_lit->matches.end(); node_iter++) {

changed_score |= epmem_unsatisfy_literal(child_lit, (*node_iter).first,
(*node_iter).second, current_score, current_cardinality, symbol_node_count);
}

However, since the child_lit->matches set can be modified in the recursive
call to epmem_unsatisfy_literal, the node_iter iterator can be left invalid
if the child_lit->matches set is now empty. Incrementing the invalid
iterator in the for loop causes the crash. If we instead replace this loop
with an explicit check to ensure that the set is not empty, we can avoid
this crash:

while (node_iter != child_lit->matches.end())
{
changed_score |= epmem_unsatisfy_literal(child_lit, (*node_iter).first,
(*node_iter).second, current_score, current_cardinality, symbol_node_count);
if (child_lit->matches.empty())
break;
++node_iter;
}

Although this fixes the crash, we still don't know the root cause of the
problem: what about the epmem query is actually causing this to crash?

[so...@googlecode.com]

Comment #3 on issue 154 by adam.syp...@soartech.com: inconsistent crash on
epmem query
http://code.google.com/p/soar/issues/detail?id=154

Sorry, forgot a line of code. Immediately before the while loop:

epmem_node_pair_set::iterator node_iter = child_lit->matches.begin();

[so...@googlecode.com]

Updates:
Status: Verified

Comment #4 on issue 154 by marin...@gmail.com: inconsistent crash on epmem
query
http://code.google.com/p/soar/issues/detail?id=154

(No comment was entered for this change.)