Signal an error when internal file descriptor limit is reached (at most 256 files can be open at the same time)

[zickgraf]

and add corresponding notes to the documentation

Rationale: I guess most code does not handle the case that InputTextFile or OutputTextFile returns fail properly, or just prints a generic error message like "Could not open file", which is not very helpful. Thus, making this error explicit significantly simplifies debugging. If one really wants to handle this gracefully, one can look at InputTextFileStillOpen and OutputTextFileStillOpen to detect if opening a new file would reach the descriptor limit.

Text for release notes

none

[fingolfin]

Fine by me, though better would of course be if we took automatically care of closing streams whenever possible. I see two ways to achieve that:

1. via finalizers
2. via weak pointers

Using finalizers requires work in the kernel: basically, whenever a stream holding a file descriptor is garbage collected, we could close its file descriptor (to avoid closing the same fd twice, CloseStream then should also make sure to do something like stream![1] := -1). But finalizers can only be set in the kernel, and only for a while TNUM range at once... One could get around that in various ways, e.g. we could add a T_FILEDESC TNUM just for this... I could go on, but let's rather look at the second option first.

We could use a "weak pointer object": we already have InputTextFileStillOpen and OutputTextFileStillOpen which hold all open file descriptors. I propose we merge them into one (not strictly necessary, but reduces the work needed and the general overhead), and add a companion WeakPointerObj which tracks for each fd the corresponding stream.. So roughly like this (pseudo code):

FileDescriptorsStillOpen := []; # TODO: probably should be thread local?
StreamsStillOpen := [];

# helper for use in stream constructors
BindGlobal("RegisterFD", function(fd, stream)
  local i;
  i := 1;
  # search for an unused slot
  while IsBound(FileDescriptorsStillOpen[i]) and IsBound(StreamsStillOpen[i]) do
    i := i + 1;
  od;
  # if a stream is gone but its file descriptor is still there, close the file descriptor
  if IsBound(FileDescriptorsStillOpen[i]) then
    CLOSE_FILE(FileDescriptorsStillOpen[i]);
  fi;
  FileDescriptorsStillOpen[i] := fid;
  StreamsStillOpen[i] := stream;
end);

# helper to use in CloseStream methods
BindGlobal("CloseFD", function(fd)
  local i;
  i := Position(FileDescriptorsStillOpen, fd);
  Unbind(FileDescriptorsStillOpen[i]);
  Unbind(StreamsStillOpen[i]);
  CLOSE_FILE(fd)
end);

That does of course not make your patch redundant, but it would reduce the impact of people forgetting to use CloseStream

[zickgraf]

I have relaxed the ErrorQuit to ErrorReturnVoid to make this less obtrusive.

[fingolfin]

Could you perhaps add a test? Something like this should work, I think:

gap> streams:=[];;
gap> for i in [1..300] do
>   Add(streams, OutputTextFile(Concatenation("test",String(i)),false));
> od;;
Error, ...
gap> Perform(streams, CloseStream);

[zickgraf]

Could you perhaps add a test?

Done!