Jump to content

Classic bug: Optional menu text file is never closed

From Galactic Empire wiki
Bug ID GE-BUG-043
Affected versions Classic 3.2e and published classic source
Status Verified in source; corrected in ge-next
Fixed in ge-next 0.1
Primary source optdisp() in GEMAIN.C

Summary

Classic GE opens the optional main-menu text file when a player selects the configured extra menu entry, but never closes it. The stream is stored in a static pointer and reused while the module remains loaded. Reaching the end of the file resets the player's reading position and returns to the main menu, but leaves the file handle open indefinitely.

This is a resource-lifetime bug rather than a failure to display the text. The optional page normally appears to work because the still-open stream can be rewound and reused.

The optional menu page

Classic's OPTMENU configuration option adds one customizable selection to the module's main menu. OPTTXT supplies its visible description and OPTCHR supplies its selection character. The default configuration describes it as:

N ... Whats New!

Selecting that entry invokes optdisp(), which reads MBMGEMNU.TXT. The routine emits one line at a time so MajorBBS can pace the output through repeated CYCLE events. Each channel has an entry in opttbl[] recording the file offset at which that user should resume.

The text reader itself is shared. Classic declares its stream pointer as a static local:

void FUNC optdisp()
{
    static FILE *hdl = (FILE *)0;

    if (hdl == (FILE *)0)
    {
        hdl = fopen("mbmgemnu.txt", "rt");
        ...
    }

The first user to select the optional entry opens the file. Later calls reuse the same stream and seek to the current user's saved offset before reading the next line.

End-of-file path

When fgets() reaches EOF, classic prepares to return that user to the main menu:

else
{
    opttbl[usrnum] = 0;
    logthis("optdisp: hit eof - fpos set back to 0");
    usrptr->substt = OPTDISP2;
    btuinj(usrnum, CYCLE);
    return;
}

Resetting opttbl[usrnum] makes that user's next visit begin at offset zero. OPTDISP2 changes the input state back to the main menu on the following cycle. Nothing in this path calls fclose(hdl), however, and no other classic shutdown or menu path closes this stream.

The static hdl consequently remains non-null and continues to refer to the open file after the display has finished.

Scope and consequences

This does not open one stream per line, user, or visit. Because the pointer is static, one continuously loaded instance of GE normally opens one stream and keeps reusing it. The immediate leak is therefore bounded at one file handle per loaded module instance.

That still has several consequences:

  • one entry in the process's limited C file table remains occupied after the optional text is no longer being displayed;
  • the operating system or C runtime may continue to regard MBMGEMNU.TXT as open, potentially interfering with replacing or editing it under DOS or Windows file-sharing rules;
  • the module has no explicit way to flush and release the stream during its own lifetime; and
  • module unload/reload or cleanup behavior can lose the static pointer without executing fclose(), potentially accumulating handles if the host runtime does not reclaim module-owned streams automatically.

The final effect depends on the host, C runtime, and file-sharing mode. A single leaked read handle may not produce an obvious gameplay symptom, which is why the optional page can appear completely functional.

Correction in ge-next 0.1

ge-next closes the shared stream when EOF is reached and immediately clears the static pointer:

fclose(hdl);
hdl = (FILE *)0;
return;

Both statements are necessary. fclose() releases the file handle; setting hdl to null tells the next call that it must reopen the file. Leaving a closed but non-null FILE * would cause the next fseek() to use an invalid stream.

The per-user offset is already reset before the close. A later selection therefore reopens the configured menu file and starts that user's display from the beginning. If another user still has a saved nonzero offset, the newly opened shared stream can seek to that offset normally. MajorBBS processes the module synchronously, so closing at EOF does not introduce concurrent access to the shared pointer.

The correction was committed during development of ge-next 0.1 and is present in ge-next and ge-next32 releases from that version onward.