IO_WriteLine fails for certain long strings

[mtorpey]

I have found that IO_WriteLine is unable to write certain long strings successfully. On two different computers I have been able to reproduce the following, where the first 2^17 characters of a long string can be written to a single line, but any longer string returns "fail":

gap> f := IO_File("bug.txt", "w");
<file fd=4 wbufsize=65536 wdata=0>
gap> s := List([1..2^18], x->CharInt(Random([63..126])));;
gap> s{[1..30]};
"DbRyD[cnrB^PKuohlarEsYG_gP}[h_"
gap> IO_WriteLine(f, s{[1..30]});
31
gap> IO_WriteLine(f, s);
fail
gap> IO_WriteLine(f, s{[1..2^17]});
131073
gap> IO_WriteLine(f, s{[1..2^17+1]});
fail
gap> LastSystemError();
rec( message := "no error", number := 0 )

I investigated IO's documentation, but couldn't find anything about a line-size limit. Furthermore, I constructed a different example which writes a much longer string successfully:

gap> a := ListWithIdenticalEntries(2^18, 'a');;
gap> a{[1..10]};
"aaaaaaaaaa"
gap> IO_WriteLine(f, a);
262145

Perhaps I have made an oversight, but for the moment I'm unable to write certain long strings to files.

[markuspf]

I quickly investigated this. What happens is that the write buffer is filled (65536 bytes) and then write actually returns success (of writing the first 65536 bytes), but because of some int32_t vs int64_t mismatch I still have to find this result is interpreted as Fail (0x1000000043e8).

I'll have to dig a little more for a fix. You can increase the size of the write buffer, but you'd still have to be careful with the Fail return values ;).

Edit: Mhm, I was wrong, the code actually does write out the first part of the buffer and then fails on the second part.

Edit2: This turns out ot be an off-by-one error in IO_Write, I'll make up a fix right away.

[markuspf]

Ok, I was wrogn again. Maybe I should do the whole debugging before posting. If you use

gap> f := IO_File("bug.txt", "w");
<file fd=4 wbufsize=65536 wdata=0>
gap> s := List([1..2^18], x->CharInt(Random([63..126])));;
gap> ConvertToStringRep(s);
gap> IO_WriteLine(f, s{[1..2^17+1]});
131074

(Which is correct because it added a newline at the end of the string). There is a case in the IO_Write function that after filling the write buffer just spills the rest of the passed string to IO_write, but IO_write expects it's input to be in IsStringRep, which it isn't if you make your String out of a list.

I am not sure what the correct fix is, either IO_Write(Line) can only take IsStringRep as input, or it has to make a converted copy for IO_Write. I have to find out what IsStringRep does to judge whether IO_write needs IsStringRep to function properly (I suspect it does).

[stevelinton]

From the documentation it looks like IO_Write needs to handle arbitrary lists of characters. It says

"All the other arguments are just written to f if they are strings. Otherwise, the String function is called on them and the result is written out to f."

There is no guarantee that the String method will produce strings in any particular representation (for instance the String of a virtual list might be virtual). So IO_Write needs to be fixed.

[markuspf]

I suppose just deleting this part of the code:

              # Perhaps we can write a big chunk:
              if Length(st)-pos > f!.wbufsize then
                  bytes := IO_write(f!.fd,st,pos,Length(st)-pos);
                  if bytes = fail then return fail; fi;
                  pos := pos + bytes;
              fi;

will do the trick. It leads to IO_Write copying the passed argument into the write buffer first, then flushing it to disk if the write buffer is full. A partially filled buffer will not be flushed unless requested.

[markuspf]

Michael, can you try the patch I submitted in the pull request #20 ?

[mtorpey]

Done. That seems to have fixed the problem.

[fingolfin]

Thanks a lot for the report, and thanks for the fix! Both are very much appreciated.

One very minor nitpick: Please don't close issues just because a PR for them is there; they should only be closed once the issue actually has been fixed in the repository.