asert in do_count about link-grammar HOT 8 CLOSED

(gdb) print id
$2 = 65524
(gdb) print d
$3 = (Disjunct *) 0x7ffdce75d660
(gdb) print d->match_id
$4 = 65512

from link-grammar.

Hmm. match_id is a uint16_t and the value here is darned close to the overflow value...

from link-grammar.

Hmm. match_id is a uint16_t and the value here is darned close to the overflow value...

I think (not validated by actual debugging but only by code inspection) that this causes the bug.

The purpose of the match_id check is to validate that there are no extra or missing pops up in the match_list stack, and also that no memory corruption occurs in it. To that end, in form_match_list() the id value is grabbed into a local variable lid, so the same value is used for all the match_list entries in the same form_match_list(), not caring whether id has been changed meanwhile by another thread.

But note line 84:

I think the fix is just to remove lines 83-85 (which exist may be due to a code rot).

On the same occasion, it is possible to save a 0 assignment in count.c. However, the code there seems to be a moving target while I'm writing this...

from link-grammar.

What about this:

That looks like a non-atomic increment to me; two threads can ++id at the same time. I believe I have 10 threads running LG at the same time.

from link-grammar.

I just now pushed this: 9f73b37 in the hope that it will narrow down or avoid the problem.

from link-grammar.

I'm confused by this statement:

I think the fix is just to remove lines 83-85 (which exist may be due to a code rot).

If those are removed, then the id does not need to be passed as an argument to any of the functions, and the id is then not set anywhere, so it will always be zero. Did you mean something else?

from link-grammar.

I'm confused by this statement:

You are right!
Only the (id != 0) should be removed. The d->match_d = id assignment should be retained.
The #ifdef/#endif should also be retained for future efficiency if VERIFY_MATCH_LIST will not be used.
So it should be:

 #ifdef VERIFY_MATCH_LIST 
 	id->match_id = id; 
 #endif

That looks like a non-atomic increment to me; two threads can ++id at the same time. I believe I have 10 threads running LG at the same time.

No problem with that, since int lid = ++id grabs a local value that is kept constant for the whole same invocation of form_match_list(). The value of the global id can continue to change, but the form_match_list() code doesn't look at it anymore in the same invocation and all the match_list elements in this invocation only use the fixed (per invocation) local copy.

from link-grammar.

Closing. Ran for two days in half-a-dozen threads, no issues arose.

from link-grammar.