View Issue Details

IDProjectCategoryView StatusLast Update
0034499FPCRTLpublic2018-12-14 11:10
ReporterAdriaan van OsAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
PlatformDarwinOSOS XOS Version10.8.5
Product Version3.0.4Product Build 
Target Version3.2.0Fixed in Version3.3.1 
Summary0034499: AssignStream doesn't report it if the executable path isn't found
DescriptionAssignStream doesn't return ProcessID -1 if the executable isn't found. Instead a positive ProcessID is returned, suggesting a valid ProcessID, for example if I try to execute ''/bin/lsxxx" instead of "/bin/ls".

I experimented with theWaitID := fpWaitPid( theProcessID, theStatus, WNOHANG); following a suggestion by Seth Grover in fpc-pascal@lists.freepascal.org of 27-07-2009 15:27. When debugging, this indeed catches the error by returning theWaitID = theProcessID and wexitStatus( theStatus) = 127.

However, when running my test program at full speed, theWaitID= 0 was returned for a non-existing executable path like ''/bin/lsxxx" .

So, I don't see how this can be solved in application code. Also, I feel that AssignStream should handle this, as the documentation says:

"The function returns the process ID of the spawned process, or -1 in case of error."
TagsNo tags attached.
Fixed in Revision40548
FPCOldBugId0
FPCTarget
Attached Files

Activities

Marco van de Voort

2018-12-13 15:18

manager   ~0112534

Last edited: 2018-12-13 15:30

View 2 revisions

IIRC the problem is that the exec() is done in a different process (after forking).

So then we would have to add a fileexists before forking? Or document that the user should do this :-)

And then probably the next report will complain it will go wrong if it exists, but is not executable ;-)

Thaddy de Koning

2018-12-13 17:18

reporter   ~0112538

AFAIK assignstream has underlying old school Assign/rewrite/reset code.
It is documented that Assign on its own does not return any error code. It is merely an advertisement...
It is the rewrite/reset part that will return an error if they fail.

In practice this means that AssignStream does not catch any errors, but the opening of the stream will have proper errors (Both in $I- state and using exceptions in $I+)

Thaddy de Koning

2018-12-13 17:30

reporter   ~0112540

Oh, both win and nix tested. <!!note!! A bit.> They do not differ much in this case. See the sources. There may be a problem on GUI applications on windows, though, since Windows GUI apps do not have stdxxx unless you create them first.

Marco van de Voort

2018-12-13 17:31

manager   ~0112541

assignstream does execute, (as you can see in the source and the reset/rewriteless example in the docs: https://www.freepascal.org/docs-html/rtl/unix/assignstream.html)

There errno is used for error detection, not the returnvalue. Still tprocess must also detect this case, so maybe it is a matter of looking how Tprocess does it.

Marco van de Voort

2018-12-13 17:34

manager   ~0112542

Thaddy. Now I understand what you mean! You are looking at streamio.assignstream, while I am looking at Unix.AssignStream, which is more like condensed unix only tprocess piping functionality.

I think Adriaan means the unix one, because he mentions process spawning.

Adriaan van Os

2018-12-13 17:45

developer   ~0112543

Unix.AssignStream, yes.

Marco van de Voort

2018-12-13 21:11

manager   ~0112550

I meanwhile checked, and tprocess does a simple fileexists before forking.

Michael Van Canneyt

2018-12-14 11:04

administrator   ~0112562

As Marco suggests, TProcess checks for existence of the file.

So, for consistency I added a fpAccess() with check on execute permission on the program file, and returns -1 if it fails. This is the best we can do.

Anything happening in the child after fork() - and that includes the call to exec - cannot be detected in assignstream(), since we have no control over when the child process actually starts executing.

Michael Van Canneyt

2018-12-14 11:09

administrator   ~0112563

Fixed documentation example ex38.pp 1529, so it actually works if you execute it from another directory. (I used that to test my fix worked)

Issue History

Date Modified Username Field Change
2018-11-02 15:47 Adriaan van Os New Issue
2018-12-13 15:18 Marco van de Voort Note Added: 0112534
2018-12-13 15:30 Marco van de Voort Note Edited: 0112534 View Revisions
2018-12-13 17:18 Thaddy de Koning Note Added: 0112538
2018-12-13 17:30 Thaddy de Koning Note Added: 0112540
2018-12-13 17:31 Marco van de Voort Note Added: 0112541
2018-12-13 17:34 Marco van de Voort Note Added: 0112542
2018-12-13 17:45 Adriaan van Os Note Added: 0112543
2018-12-13 21:11 Marco van de Voort Note Added: 0112550
2018-12-14 11:04 Michael Van Canneyt Fixed in Revision => 40548
2018-12-14 11:04 Michael Van Canneyt Note Added: 0112562
2018-12-14 11:04 Michael Van Canneyt Status new => resolved
2018-12-14 11:04 Michael Van Canneyt Fixed in Version => 3.3.1
2018-12-14 11:04 Michael Van Canneyt Resolution open => fixed
2018-12-14 11:04 Michael Van Canneyt Assigned To => Michael Van Canneyt
2018-12-14 11:04 Michael Van Canneyt Target Version => 3.2.0
2018-12-14 11:09 Michael Van Canneyt Note Added: 0112563
2018-12-14 11:10 Adriaan van Os Status resolved => closed