#126 Using COPYDEF of OUTPUT can crash

[David Costanzo]

COPYDEF can be used to create a new name for an
existing procedure. However if you use a COPYDEF of
OUTPUT, it can crash when its used. So far, I have
only seen this in contrived cases, where the COPYDEF
was called within the procedure where it was used. I
have been able to use a COPYDEF of OUTPUT successfully
in other scenarios.

How Reproducible:
Every Time

Steps to Reproduce:
TO MYPROC
COPYDEF "MYOUTPUT "OUTPUT
MYOUPUT 123
END

SHOW MYPROC

What Happens:
FMSLogo crashes

Expected Result:
SHOW MYPROC prints 123.

[David Costanzo]

Logged In: YES
user_id=109252

I have checked in a further mitigation for this bug, which
will be available in FMSLogo 6.11.0.

The fix was copied in from UCBLogo, but that fix leaks
memory. The problem is that the code doesn't "re-treeify" a
body node if the definition of a function changes in
mid-execution, which could crash if the definition changed
in a significant way for tree-ification (such going from
undefined to defined, or changing the number of inputs).

UCBLogo "leaks" the tree-ificiation of the procedure, which
is not a problem for UCBLogo, because it has true garbage
collection. But this results in a very significant memory
leak for FMSLogo. My stop-gap fix for this memory leak is
to untree-ify the procedure after it is executed.
Unfortunately, this means that the original bug still exists
for functions that are tree-recursive (non-tail recursive)
because once the inner call exits, the outer call becomes
untree-ified and can no longer handle having a procedure change.

Since the current change reduces the scope of the bug, I
have submitted it. I'm not sure what the final fix will
be--it may either be a reference count on the tree-ification
or simply to keep the tree-ification around for the lifetime
of the procedure.

[David Costanzo]

Logged In: YES
user_id=109252

The strategy that I mentioned in the previous comment had a
noticable, deterimal effect on performance, so I have
instead adopted a different strategy that is closer to what
UCBLogo does.

Now, the code untreeifies a procedure in three places:
1) When the workspace generation changes.
2) When a procedure is erased.
3) When FMSLogo is shut down.

This "lazy evaluation" has the added benifit of not
treeifing a procedure every time it is called. It has the
drawback of increasing the working set for FMSLogo.

This change will be available in FMSLogo 6.11.0.

[amicheler]

Logged In: YES
user_id=1303092
Originator: NO

Hallo David,

do you have a solution for this bug (#1454113) already?
I have the same problem in aUCBLogo.

Thanks,
Andreas

[David Costanzo]

Logged In: YES
user_id=109252
Originator: YES

No, I haven't fixed this bug. I got COPYDEF to work in most cases, but it still crashes/hangs when a COPYDEF of OUTPUT is used in a tree-recursive procedure. (This is the only test case that is commented out in src\test\copydef.lgo).

I'm surprised that this is a problem for aUCBLogo because I thought aUCBLogo was based on the most recent UCBLogo engine, and I didn't think that UCBLogo had this bug.

[amicheler]

Logged In: YES
user_id=1303092
Originator: NO

I got this to working last night,
so it's not important for aUCBLogo any more.

I thinks I had destroyed
the working UCBLogo copydef-eval implementation some time before.
Now it is fixed in aUCBLogo again (was pretty hard).

Thanks,
Andreas