View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0034499 | FPC | RTL | public | 2018-11-02 15:47 | 2018-12-14 11:10 |
| Reporter | Adriaan van Os | Assigned To | Michael Van Canneyt | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Platform | Darwin | OS | OS X | OS Version | 10.8.5 |
| Product Version | 3.0.4 | Product Build | |||
| Target Version | 3.2.0 | Fixed in Version | 3.3.1 | ||
| Summary | 0034499: AssignStream doesn't report it if the executable path isn't found | ||||
| Description | AssignStream 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." | ||||
| Tags | No tags attached. | ||||
| Fixed in Revision | 40548 | ||||
| FPCOldBugId | 0 | ||||
| FPCTarget | |||||
| Attached Files |
| ||||
|
|
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 ;-) |
|
|
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+) |
|
|
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. |
|
|
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. |
|
|
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. |
|
|
Unix.AssignStream, yes. |
|
|
I meanwhile checked, and tprocess does a simple fileexists before forking. |
|
|
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. |
|
|
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) |
| 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 |
