I7 compiler bug with translation of "the current action is"?

I think I’ve run into a bug that causes a memory leak on the heap in some cases. The bug appears to be at the level of the I7 compiler, unfortunately. Following is a sample scenario to demonstrate:

"Leaky Topics"

Place is a room.

Every turn:
    if the current action is jumping: [this construction causes a leak]
	    say "You just jumped."

Test me with "showheap / say hello / showheap / say goodbye / showheap".

Each execution of the test me commands results in some of the heap being lost to the leak. Eventually, the heap will be exhausted, and the game will halt with an RTP.

The quick workaround, related to the conversation on an old thread (Performance tip: avoid "current action"), is to change the phrasing of the if condition:

Every turn:
    if jumping: [this construction does not cause a leak]
	    say "You just jumped."

As for the root cause, the issue is that the first version gets translated into I6 as the following:

((( BlkValueCompare(STORED_ACTION_TY_Current(I7SFRAME), TryAction(0, player, ##Jump, 0, 0, STORED_ACTION_TY_Current((I7SFRAME+WORDSIZE)))) == 0)))

This code sets up two stored actions on the block value stack: the first is a copy of the current action, the second is a stored action conforming to the first five parameters in the TryAction() call.

The problematic part is the sixth parameter of the TryAction() call, which is intended to be the address of the stored action to which the first five parameters should be applied. The fact that this parameter takes the form of a second call to STORED_ACTION_TY_Current() seems to be an error – it does not seem necessary for the intended purpose, which would be served just as well by:

((( BlkValueCompare(STORED_ACTION_TY_Current(I7SFRAME), TryAction(0, player, ##Jump, 0, 0, (I7SFRAME+WORDSIZE))) == 0)))

Note that a wrapper function will have already set up a default stored action on the stack at I7SFRAME+WORDSIZE.

If the current action involves a topic as one of its parameters, e.g. the answering it that action used above, then the details of STORED_ACTION_TY_Current() cause a TEXT_TY block value to be set up on the heap to hold the text of the topic understood. The call to TryAction(), if given a sixth parameter, results in only this (the first) line being executed:

if (stora) return STORED_ACTION_TY_New(ac, n, s, by, req, stora);

The STORED_ACTION_TY_New() routine contains the line:

BlkValueWrite(stora, STORA_COMMAND_TEXT_F, 0);

This line severs the link between stored action at I7SFRAME+WORDSIZE and the text block value created by the call to TryAction() as a place to hold the copy of the topic understood text. When the stored action block value on the stack is freed, the orphaned text block value is left intact, causing the leak.

It took me quite a while to hunt down this memory leak on a Z-Machine game, so I thought it worth documenting for posterity. The leak is a less serious issue under Glulx, because the VM can easily obtain enough additional heap memory at run-time to mask its presence for a very long time.

3 Likes

It is possible to compensate by adjusting STORED_ACTION_TY_New() to check for the presence of an existing value at STORA_COMMAND_TEXT_F and to free that value if it is found:

Include (-

! ==== ==== ==== ==== ==== ==== ==== ==== ==== ====
! StoredAction.i6t: Setting Up (modified)
! ==== ==== ==== ==== ==== ==== ==== ==== ==== ====

[ STORED_ACTION_TY_New a n s ac req stora flag; ! MODIFIED
    if (stora) flag = true; ! ADDED: notice if an existing stored action is supplied as the target
    if (stora == 0) stora = BlkValueCreate(STORED_ACTION_TY);
    BlkValueWrite(stora, STORA_ACTION_F, a);
    BlkValueWrite(stora, STORA_NOUN_F, n);
    BlkValueWrite(stora, STORA_SECOND_F, s);
    BlkValueWrite(stora, STORA_ACTOR_F, ac);
    BlkValueWrite(stora, STORA_REQUEST_F, req);
    if (flag) STORED_ACTION_TY_Destroy(stora); ! ADDED: free any pre-existing stored command text
    BlkValueWrite(stora, STORA_COMMAND_TEXT_F, 0);
    return stora;
];

-) instead of "Setting Up" in "StoredAction.i6t".

The above seems to compensate for the issue, but I’m not 100% sure that it doesn’t have any unanticipated side effects. It’s working in the project where I encountered the leak.

Reported as I7-2119

2 Likes

Much obliged, as always, Zed!

1 Like