#363 Heap corruption when halting evaluation of default params

[DanielAjoy]

I load this program

;===================
to ini
show nodes
make "posible.sol runresult [catch "found [try]]
end

to try [:a iseq 1 10000]
repeat 10 [
if repcount = 9 [(throw "found 1)]
(try)
]
end
;===================

I open the STATUS pop-up window

I type INI in the command line. I let "peak memory" reach about 191000

I press the STOP button.

I type

show nodes
[40524 190580]
show nodes
[40524 40528]
show nodes
[40524 40528]

So it seems it has stabilized at 40524

I type INI again.

I let "peak memory" reach about 250000 and then press the STOP button again

Then:

show nodes
[100525 250609]
show nodes
[100525 100529]
show nodes
[100525 100529]
show nodes
[100525 100529]

So ¿more than twice as many nodes in use? ... but the program has no global variables.

[David Costanzo]

When I press HALT on a debug build of FMSLogo, it complains that it's manipulating freed memory, which makes me think that the memory leak you observe is the best possible symptom. I'll need to look into this some more.

[David Costanzo]

I have checked in a fix for the memory corruption, but that was independent of the leak which was described by this bug. I still need to investigate that, but I'm hoping that it's a duplicate of bug #1568629.

The fix for the memory corruption will be available in FMSLogo 6.29.0.

[David Costanzo]

I have checked in a fix for this. The fix will be available in FMSLogo 6.29.0

The memory leak was related to the crash. In short, the evaluation engine has "unreferenced registers" which need to be backed up and restored as it evaluates a function call. The routines which it used to backup/restore a NODE* which points to a location in the variable stack would inadvertently increment or decrement the reference count. In the case of the crash, the problem was that the variable stack had already been freed by the time the reference count was decremented, which caused memory corruption. In most other cases, the operation would simply add a reference count to the variable that represented the parameter, causing it to leak the variables value. In your case, this leaked the output of "ISEQ 1 10000" each time TRY was called.

Note that it's unlikely that you'd ever see the crash in a release version of FMSLogo because "freed" NODEs get kept around for later re-use and decrementing the reference count of such a NODE is safe (unless the NODE happens to get reused in the meantime).

With the fix, the number of NODES reported snaps down to within a few hundred of what INI reports before calling TRY. The additional nodes is a one-time cost associated with how FMSLogo caches the result of parsing INI and TRY.

[Nobody/Anonymous]

thanks a lot! much appreciated.

[David Costanzo]

I have tracked this back to a regression that I introduced in FMSLogo 6.8.0 (eval.cpp#16). The comment that I attached to the change indicates that I was made the change to add type safety and because UCBLogo did the same thing. I guess that, at that time, I didn't know that UCBLogo uses a different garbage collection strategy than FMSLogo and that it wasn't safe to make FMSLogo code look like UCBLogo.