View Issue Details

IDProjectCategoryView StatusLast Update
0027221FPCRTLpublic2018-02-10 15:45
ReporterAnton Rzheshevski Assigned ToFlorian Klämpfl  
PrioritynormalSeverityminorReproducibilitysometimes
Status resolvedResolutionfixed 
PlatformWin32OSWine 
Product Version2.6.4 
Summary0027221: TFileStream.Destroy fails to free the file handle, blocking any further access to that file if Handle was 4
DescriptionHappens when Handle is 4 because of the following code in RTL:
Procedure FileClose (Handle : THandle);
begin
  if Handle<=4 then
   exit;
  CloseHandle(Handle);
end;
Due to this, when I try to close a file then open it later again, my program crashes.
Steps To Reproducevar s: TFileStream;
s:= TFileStream.Create(FileName, fmOpenReadWrite);
//Check here if s.Handle is 4. Annoyingly, it is.
s.Free;
s:=TFileStream.Create(FileName, fmOpenReadWrite); //this one fails;
s.Free;

Happens when I run my application from any terminal emulator using any of the following lines:
>wine ./myexe.exe
>./myexe.exe

Never happens when I run my application from any file manager (gnome commander, midnight commander, wine explorer.exe and so on)
Additional InformationMay be other factors at work as my game engine performs many other operations at startup, including opening kernel32.dll for read to detect Wine by its signature string.
TagsNo tags attached.
Fixed in Revision 38189
FPCOldBugId
FPCTarget
Attached Files

Relationships

has duplicate 0033135 resolvedMarco van de Voort Bug 0027221 is still not fixed in the 3.x branch 

Activities

Dmitriy Pomerantsev

2014-12-30 13:31

reporter   ~0080023

Looks like a protection for standard predefined console streams (stdin, stdout, stderr), but their numbers 0..2, not 3 and 4. Windows also no need to close INVALID_HANDLE_VALUE which is not checked here.

Michael Van Canneyt

2014-12-30 15:53

administrator   ~0080028

The handles are not 0..2, this is only on Unixes.
On windows these handles are in standard variables in the system unit: StdInputHandle, StdOutputHandle StdErrorHandle

If a check is needed (which I doubt, there are cases when you do want to close the stdin/out/err handles) then these handlers should be checked.

However, this needs to be checked by someone using windows.

Michael Van Canneyt

2014-12-30 15:53

administrator   ~0080029

I am assigning it to Florian, I think this should be fixed before branching 2.8.0/3.0.0

Dmitriy Pomerantsev

2014-12-30 16:09

reporter   ~0080031

Last edited: 2014-12-30 16:10

View 3 revisions

Yes, we investigate this at freepascal.ru. BTW, Std*Handle in windows is NOT 0..2, because fpc have use GetStdHandle() itself and according to http://msdn.microsoft.com/ru-RU/library/windows/desktop/ms682075%28v=vs.85%29.aspx the function result is different to 0..2. For example my StdOutputHandle in today tests was 7.

Now I think it will be better to remove any checking except INVALID_HANDLE_VALUE. Because of THandleStream:

InputStream := THandleStream.Create(GetStdHandle(STD_INPUT_HANDLE));
OutputStream := THandleStream.Create(GetStdHandle(STD_OUTPUT_HANDLE));

If anyone have to use TFileStream to access to standard stream he must have a very good reason for it.

Anton Rzheshevski

2018-02-08 14:13

reporter   ~0106285

Someone please update its "Product Version" to 3.0.4 -- I fear it would never get fixed simply because it is assigned to 2.6.4.

Marco van de Voort

2018-02-08 15:00

manager   ~0106287

I guess to come to a solution, the question is for which scenario that check was added. It is old, pre SVN times.

Anton Rzheshevski

2018-02-08 20:50

reporter   ~0106298

Last edited: 2018-02-08 22:27

View 3 revisions

I looked through FPC 0.99.14 sources dated 2000 in "really old files" at sourceforge and my thoughts are "erroneously dragged from MS-DOS code in the ancient times predating even Windows XP".

Let's first lok at the primeval form of this procedure. Note the empty line!

win32:
Procedure FileClose (Handle : Longint);

begin
  if Handle<=4 then
   exit;
  CloseHandle(Handle);
end;

now, let's take a look at go32v2:
Procedure FileClose (Handle : Longint);
var Regs: registers;
begin
  if Handle<=4 then
   exit;
Regs.Eax := $3e00;
Regs.Ebx := Handle;
RealIntr($21, Regs);
end;

Looks like copy-paste, yes?
Especially their identic left padding and the empty line (collapsed in later FPC versions) where "var Regs: registers" was in the DOS version.

P.S. The Linux version is almost identic to win32 one:
Procedure FileClose (Handle : Longint);

begin
  if Handle<=4 then
   exit;
  fdclose(Handle);
end;


while modern fpc 3.0.4 has unix version updated to:
Procedure FileClose (Handle : Longint);
var
  res: cint;
begin
  repeat
    res:=fpclose(Handle);
  until (res<>-1) or (fpgeterrno<>ESysEINTR);
end;

the Win32 version remained unchanged.

P.P.S I checked both modern MSDN for CloseHandle and "The Peter Norton Programmer's Guide to the IBM PC" (I have a Russian translation from 1992) for Int21 function 3E. There are no mentions of any "special" cases.

Marco van de Voort

2018-02-10 14:55

manager   ~0106315

Thanks for the investigation, I remove the check for win32/64

That leaves dos still open, but maybe it is better to file something separate for that.

Marco van de Voort

2018-02-10 15:44

manager   ~0106320

Last edited: 2018-02-10 15:45

View 2 revisions

Seems that dos really has 5 standard handles (according to the dos devels):

0 - stdin, 1 - stdout, 2 - stderr, 3 - stdaux (AUX:), 4 - stdprn (PRN:),

Issue History

Date Modified Username Field Change
2014-12-30 10:32 Anton Rzheshevski New Issue
2014-12-30 13:31 Dmitriy Pomerantsev Note Added: 0080023
2014-12-30 15:53 Michael Van Canneyt Note Added: 0080028
2014-12-30 15:53 Michael Van Canneyt Assigned To => Florian Klämpfl
2014-12-30 15:53 Michael Van Canneyt Status new => assigned
2014-12-30 15:53 Michael Van Canneyt Note Added: 0080029
2014-12-30 16:09 Dmitriy Pomerantsev Note Added: 0080031
2014-12-30 16:10 Dmitriy Pomerantsev Note Edited: 0080031 View Revisions
2014-12-30 16:10 Dmitriy Pomerantsev Note Edited: 0080031 View Revisions
2018-02-07 20:26 Marco van de Voort Relationship added related to 0033135
2018-02-07 20:26 Marco van de Voort Relationship replaced has duplicate 0033135
2018-02-08 14:13 Anton Rzheshevski Note Added: 0106285
2018-02-08 15:00 Marco van de Voort Note Added: 0106287
2018-02-08 20:50 Anton Rzheshevski Note Added: 0106298
2018-02-08 22:09 Anton Rzheshevski Note Edited: 0106298 View Revisions
2018-02-08 22:27 Anton Rzheshevski Note Edited: 0106298 View Revisions
2018-02-10 14:55 Marco van de Voort Fixed in Revision => 38189
2018-02-10 14:55 Marco van de Voort Note Added: 0106315
2018-02-10 14:55 Marco van de Voort Status assigned => resolved
2018-02-10 14:55 Marco van de Voort Resolution open => fixed
2018-02-10 15:44 Marco van de Voort Note Added: 0106320
2018-02-10 15:45 Marco van de Voort Note Edited: 0106320 View Revisions