#1 2018-10-25 20:51:33

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,571
Website

IO error handling when working with streams (CRITICAL)

I'm discover critical issue in SynCommons (actually in all other units also SynBz, SynCrypto, SynEcc, .....) while working with streams.
All streams writings in mORMot are use Stream.write(buf, size) without checking actual operation result.
If IO operation is not success then Stream.write return 0 (which means Error) but this not checked and exception is not raised.

My suggestion is to replace ALL calls to (f)Stream.Write -> stream.WriteBuffer for all units. It's simpler when adds check everywhere IMHO.

Real case - disk is almost full (still can create file but can't write to it something long)
Code below run without any errors and create a zero length files  ;(

    stream := TFileStream.Create(filePath, fmCreate);
    writer := SynCommons.TTextWriter.Create(stream, 65536);
    try
      writer.add*****
    finally
      writer.FlushFinal;
      writer.Free;
      stream.Free;
    end;

@ab - this is really very serious problem

Offline

#2 2018-10-26 06:59:04

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,670
Website

Re: IO error handling when working with streams (CRITICAL)

Is WriteBuffer compatible with FPC and oldest Delphi?

Offline

#3 2018-10-26 07:39:54

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,571
Website

Re: IO error handling when working with streams (CRITICAL)

With FPC - yes. FPC implementation placed below. Don't know about old Delphi

procedure TStream.WriteBuffer(const Buffer; Count: Longint);

  var
    r,t : Longint;

    begin
      T:=0;
      Repeat
         r:=Write(PByte(@Buffer)[t],Count-t);
         inc(t,r);
      Until (t=count) or (r<=0);
      if (t<Count) then
         Raise EWriteError.Create(SWriteError);
    end;

Offline

#4 2018-10-26 20:45:32

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,670
Website

Re: IO error handling when working with streams (CRITICAL)

Please check https://synopse.info/fossil/info/351e38c844

I've tested it in Delphi 5 (!) and up, and with FPC.
Sounds be good enough at first attempt.

Offline

#5 2018-10-27 12:09:28

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,571
Website

Re: IO error handling when working with streams (CRITICAL)

I've test it carefully in different scenarios (for tests I create a small file based file system using mkfs and mount - I love Linux).

TTextWriter with streams now work as expected and throws in case no disk space left (prev. implementation creates 0 length or corrupted files). This is VERY good.

But now there is another problem - logging (SynLog). If disk quota exceed exception inside logging process now broke all server logic.
Common pattern for logging is to stop write logs and give a chance to system administrator to add some disk space without broke application.

In most case application is still can work even if can't write logs because on the production environment database (even if on the same server) usually write his files to another partition.

I try to fix SynLog by adding try/catch inside all TSynLog.LogInternal but this didn't help (in any case this is ugly solution).

Before IO fix writer inside SynLog simply ignore IO errors and everything works as expected for logging - while there is no space on the disk logs are simply not written, when I free some space, they are written again.

More complex fix to TTextWriter is required, may be:

  - add  some property IgnoreIOErrors (default false) and handle Stream.write() results manually
  - Or create our own TFileStream descendant what able to ignore IO errors inside Write (WriteBuffer is not virtual) and use it inside TSynLog for fWriterStream instance - this will be simple solution

Offline

#6 2018-10-27 12:18:02

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,571
Website

Re: IO error handling when working with streams (CRITICAL)

Yes, TFileStream descendant seems good idea. I'll prepare the patch now

Offline

#7 2018-10-27 12:36:14

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,571
Website

Re: IO error handling when working with streams (CRITICAL)

Please, see 145 pull request

Offline

#8 2018-10-27 13:08:47

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,670
Website

Re: IO error handling when working with streams (CRITICAL)

Adapted as https://synopse.info/fossil/info/4ef2b787c0

Hope it works as expected.

Offline

#9 2018-10-27 13:39:16

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,571
Website

Re: IO error handling when working with streams (CRITICAL)

Thanks! Work as expected for me.
I do not use LZ'ipping during log rotation, but potentially unexpected error can be there (as far as I understand before IO fix rotation created broken LZ if out of disk, but now will throw)

Offline

Board footer

Powered by FluxBB