I posted this on os4coding.net already, but I'll repost here in the hope that @colinw sees it.
I have some code that launches a child process and then reads its output. It's working fine on OS3, but on OS4, both the m68k and PPC builds crash when I call IDOS->Read(). I've written below a simplified version (with error checking removed) of the code. Can anyone spot anything silly that I'm doing wrong that would cause this crash?
Where is the child process exit arbitration code ?
Who is handling the allocated resources ?
You can't just create a new distinct child process and have it go hurtling into
code that was loaded and unloaded by another process at any time without
some sort of exit or resource arbitration.
The exitcode() for example is INSIDE the loaded segment for the parent
process that is calling createnewproc(), not the child segment itself,
what will happen if the parent exits and unloads before the child process does. ?
The exitcode() code will be running in someone elses freed memory.
Also, who's closing the streams and unloading the seglist - and when ?
I can't see any of that, so I can't comment on this aspect.
You need a propper exit arbitration method at least.
There are examples in the CreateNewProc() autodoc.
This low level function requires a lot of care and understanding about
process context, ie. which process is doing what and when.
The puzzling question is; why are you not just using a propper shell-handler
process to take care of all the semantics and resources ?
Just use Systemtags(), that's what it's for.
I have carefully read the AutoDocs and believe that the function is called correctly. There is a small bug in that I don't unload the segment after the child process has exited as I thought this was done automatically. I can fix that, but it's not the cause of the crash.
You can see from the code that the function waits for the child to exit, so there should be no problem with the parent exiting before the child.
But it is crashing and I'd really like to know why.
@gdridi, I checked and my buffer is aligned to a multiple of 16 bytes, and I tried using MODE_READWRITE but that doesn't help.
I'm sorry, but I don't even know where to begin.
Firstly, it is never wise to mix DOS calls with CLIB calls, one doesn't know what the other
is doing with its streams and such.
Here's an initial problem I can see, the pipe streams are being opened with DOS calls,
they are being passed to a DOS function, CNP(), all good so far...
but here is where the trouble starts.
The disk based executable is LoadSeg()'ed and passed directly to the low level CNP() call.
I have no idea what kind of startup code is in there, do you ?
Is it expecting CLIB, NEWLIB, DOS _start() only ? (see; include/dos/startup.h)
If it's either of the first two, (CLIB/NEWLIB), then having NP_Cli,TRUE set will make the code
think it's being run under a fully features shell-handler process with all the associated
shell features, such as command redirection arg parsing, default streams and locks etc...
With no CLI structure, it would assume it is being started from Workbench and would have
to grab a workbench startup message waiting at the process message port and then do other
stuff with that instead.
Here's the part I am try to be clear about.
Having just a CLI structure stuck onto a process struct, does NOT make it a shell process.
It's no longer interpreted as a Workbench process but it isn't a Shell process either.
It is just a low level DOS process with a CLI structure hanging off it, nothing more.
There are warnings about this with that tag NP_Cli.
So the unanswered questions are;
* What is in that loadable and what is it doing ?
* What sort of environment is it expecting to be run in ?
* What is it doing with the stream descriptors ?
* Does the startup code (if any) expect paths, current dir locks and such like "shell stuff" ?
* Is it expecting any shell arguments and how are they being parsed ? ReadArgs() ?
* Worst of all, where is it expecting the arguments to come from ?, the CNP() call
doesn't provide any NP_Arguments to the start() function whatsoever, infact it
will only see a NULL pointer and that will make IDOS->ReadArgs() break completely.
As I mentioned before, CNP() is a _LOW LEVEL_ building block function,
use SystemTags() or the CLIB system() function, otherwise you will be playing
Whack-A-Mole trying to basically recreate a pseudo shell-handler environment before
all the possible environment and compatibility problems go away.
There are only three possible DOS startup methods expected with all loadables.
They are at the bottom NOTES section of include/dos/startup.h
This is only the application startup detection method, it DOES NOT mean
that sticking a CLI on a process turns it into a shell-handler process,
as mentioned above, other things are required.
Ok, you've convinced me that this is not the function I want, so let's change our approach.
First, let's get a big loud warning right at the top of the CreateNewProc() AutoDocs saying "This is probably not the function you want" and ensure that it is made clear that it is a specialised function (that you might use for creating threads for instance) and that for launching external applications, you need to use SystemTagList(). In your reply in this thread, you've given a great explanation of CreateNewProc() - information that I couldn't pick up from the AutoDoc. So let's get some more of that good information into the AutoDoc entry for future reference.
I wrote this code many years ago, and I do remember at the time being a bit confused about what function I should be using. If I remember correctly, I couldn't get my use case working with SystemTagList(). For your information, here is my use case.
I have an application that helps with Amiga cross-development. You compile your Amiga OS app on a PC and then use my helper application to send it to a remote Amiga (which could be running OS3 or OS4) and execute it there. The series of steps is as follows:
1. Start the remote application development server (RADRunner) on the Amiga
2. Compile the software on a PC and send the binary to the remote Amiga (where RADRunner is listening on a socket)
3. On the Amiga, RADRunner uses CreateNewProc() or SystemTagList() to launch the app asynchronously
4. The application is launched as a CLI process, and it outputs text to stdout
5. RADRunner on the Amiga loops around, capturing stdout coming from the app, and returning it to the PC via a socket, again, asynchronously
6. The application under test on the Amiga terminates, causing RADRunner to terminate its loop, clean up resources and return the app's exit code to the PC
If I remember correctly, I had trouble getting the app to run asynchronously on the Amiga, while simultaneously reading its stdout and sending that stdout to the PC. That's why I ended up using CreateNewProc().
Do you think this use case can work with SystemTagList()?
I can't see any immediate reason why it can't be done.
All you need to remember is that if you can do it in a shell commandline,
(maybe with run), you can use the same basic commandline with SystemTags(),
that comes in handy when trying to debug something like this.
It's just the streams that you need to take care of.
Also, while I remember, the shell-handler has built-in piping,
you don't need to mount the queue-handler for PIPE:
the s:shell-startup _mchar and _pchar can be used directly
on the commandline for the streams.
Some aspects of the streams can be somewhat daunting as they
are not explained well in previous SDK:autodocs for SystemTags()
Recently after someone else bringing it to my attention,
I have rewritten it and added some more "helper" tags to make
it clearer, this new document will go out in the next public sdk,
it may make it easier to understand the semantics using SYS_ASynch
mode somewhat easier (I hope).
I will post the new System() autodoc in the next reply.
I have also added a new autodoc to the (internal) shell-handler
pipe function, this may help you understand how to format a
commandline to achieve what you want, this is normally done
internally within the shell, but I thought the document would help
to understand how it works and there wasn't anything public yet.
/****** commands/PIPE ********************************************************
*
* NAME
* PIPE - Connect input and output streams of shell commands
*
* FORMAT
* PIPE <command>
*
* TEMPLATE
* COMMAND/A/F
*
* PATH
* Internal
*
* FUNCTION
* The PIPE command function is used internally by the shell to connect
* the input and output streams of shell commands. It is not useful
* beyond that point and this 'pipe' command should not be used by
* user shell scripts directly.
*
* EXAMPLE
* For use, something like this needs to be in your S:Shell-Startup file;
* set _mchar "||"
* set _pchar "|"
*
* Note that these must be local variables or things won't work.
* Once you have set these up, you can send the output of one program
* to another one's input.
*
* The two different separator characters have different purposes.
* The pchar "|" is to separate individual command sections whose
* input and output streams are wired together, as in;
* list | grep x
*
* The mchar "||" is for wiring the output of two programs together,
* the input streams are unaffected, as in;
* list || type s:foo
*
* Some programs don't accept input or output unless its names are
* placed on the command line. For those you can use the pseudo-devices
* "IN:" and "OUT:" as substitutes for the piped input and output streams;
* list to out: | sort in: to out:
* (The sort command may be slightly broken and disallow this yet.)
*
* WARNING
* This PIPE command may be removed in a future shell versions.
*
******************************************************************************
/****** dos.library/System ************
*
* NAME
* System -- Have a shell handler execute a command line. (V36)
*
* SYNOPSIS
* int32 error = System(CONST_STRPTR command,
* const struct TagItem *tags);
*
* int32 error = SystemTagList(CONST_STRPTR command,
* const struct TagItem *tags);
*
* int32 error = SystemTags(CONST_STRPTR command, uint32 Tag1, ...);
*
* FUNCTION
* System() spawns a new Shell handler process to execute the command,
* returns the return code the command produced, or -1 if the command
* could not be run for any reason.
* Normal Shell command-line parsing will be done including redirection
* on the command.
*
* This function has two primary modes, Synchronous and Asynchronous,
* this is controlled by the boolean tag; SYS_Asynch.
* Asynchronous commands cause this function to return immediately,
* while Synchronous commands will wait until command execution finishes.
*
* Each mode has different requirements and ramifications when dealing
* with the filehandles. It is very important to follow these rules;
*
* () Synchronous execution will NOT close any filehandles when the
* command returns, if supplying your own via the tags SYS_Input,
* SYS_Output or SYS_Error, you must close them (if needed).
*
* If a tag is NOT specified, it will default to using the calling
* process' respective filehandle, ie; Input(), Output(), ErrorOutput().
*
* () Asynchronous execution WILL close both input and output filehandles
* and conditionally the error filehandle (depending on SYS_CloseError)
* after executing the command, even if these were the caller process'
* inherited Input() and Output() filehandles, so be well warned !!
*
* To stay out of trouble in Asynch mode, both SYS_Input and SYS_Output
* should generally be specified and should not be allowed to default
* to the inherited parents' filehandles, unless this is intentional.
*
* It is legal to use DupFileHandle() on the parent filehandles,
* but check the result as it is possible to have duplication failure
* and the result will be 0, which is "NIL:" and may not be intended.
*
* The ErrorOutput() stream is never inherited in Asynch mode,
* as the default is ZERO.
* The SYS_Error tag is required for anything else and also the
* SYS_CloseError tag must also be used if you want it to be closed.
*
* () If command output is not required, it is always safe to pass
* ZERO to both the SYS_Input and SYS_Output tags, this results in
* the function using "NIL:" for the shell handler process. (V53.65+)
*
* () If input and output are to both be to the same CON: window,
* pass a filehandle for a CON: window in SYS_Input and then pass a
* SYS_Output of ZERO. The shell handler will automatically set the
* default Output() filehandle to the window you passed via SYS_Input.
*
* () Specifying a SYS_Error filehandle is optional and won't result in
* loss of output as software such as CLIB and DOS functions will
* default to using the Output() filehandle when ErrorOutput() is ZERO.
* Functions like PutErrStr() and PrintFault() mention this.
*
* () NEVER under any circumstances, should you pass the same filehandle
* to more than one tag. SYS_Input, SYS_Output and SYS_Error must
* never share the same non-zero filehandle in any execution mode.
*
*
* A copy of the current directory and path list will be inherited from
* the calling process by default, if it has one, otherwise tags below
* allow for alternatives. The path and current directory are used to
* find the command for execution.
*
* This function normally uses the resident boot shell-handler,
* but other shells can be specified via SYS_UserShell and SYS_CustomShell.
* Normally, things from a user are sent to the UserShell.
* The UserShell defaults to the same shell as the boot shell handler.
*
* INPUTS
* command - (STRPTR) Program name and arguments to start as from a shell.
*
*
* TAGS
* SYS_Input (BPTR) -- Input filehandle; defaults to the parent.
* Passing a ZERO value with this tag will cause
* this function to internally open "NIL:" (V53.65+)
*
* SYS_Output (BPTR) -- Output filehandle; defaults to the parent.
* Specify ZERO for a copy of the SYS_Input stream.
*
* SYS_Error (BPTR) -- Error Output filehandle; (mode dependant),
* For SYS_ASynch,FALSE - defaults to the parent.
* For SYS_ASynch,TRUE - defaults to ZERO.
* This is not closed unless SYS_CloseError,TRUE
*
* SYS_CloseError (int32; boolean) -- Due to legacy compatibility issues
* DOS is unable to allow the SYS_Error stream to be closed by default.
* You must specify this tag as TRUE to have it automatically close
* the SYS_Error filehandle on exit, if one is supplied.
*
* SYS_Asynch (int32; boolean) -- If TRUE, this function will return
* as soon as the child shell handler process has been started.
* If FALSE, this function will wait for the shell command to finish.
* Defaults to FALSE.
*
* SYS_ExecuteInputStream (int32; boolean) -- When TRUE, the 'command'
* string parameter will be ignored, (set it to "" or 0),
* and then the input file handle will be used to read shell commands,
* this filehandle is supplied via the SYS_Input tag.
* SYS_Asynch tag will currently be forced to FALSE when this tag
* is specified, but do not depend on this behaviour for the future.
* The input file handle may also be a console descriptor to
* spawn a new syncronous interactive shell process. (V53.45+)
*
* SYS_UserShell (int32; boolean) -- TRUE make the user resident "Shell"
* execute the command, while FALSE makes a custom shell specified
* by SYS_CustomShell execute the command.
* If no SYS_CustomShell is specified, "BootShell" will be the name
* of the default resident handler.
* If this tag is TRUE, the SYS_CustomShell tags will be ignored.
* Defaults to FALSE.
*
* SYS_CustomShell (STRPTR) -- Make this specified shell handler execute
* the program instead of the default. The parameter must be the
* name of the shell handler to use, as found on the dos resident
* segment list. The function IDOS->FindSegment() is used internally
* for this, to obtain the seglist to the specified shell handler.
* This tag is ignored if the tag 'SYS_UserShell,TRUE' was specified
* Defaults to "BootShell".
*
* SYS_CurrentDir (BPTR) -- Current directory for the shell process.
* Defaults to a DupLock() copy of the parent's current directory.
*
* SYS_StackSize (int32) -- The stack size the shell process should use.
* Defaults to the DOS prefs minimum value or the same as the parent
* task (whatever is larger) when this tag is not specified.
*
* SYS_Name (STRPTR) -- Name of the shell process.
* Defaults to "Background CLI" if no tag is specified.
* The shell process' task structure; task->tc_Node.ln_Name will
* point to the DOS allocated buffer to where this string is copied.
* The exec function FindTask() searches on this string.
*
* SYS_CopyVars (int32; boolean) -- Indicate whether the local variables
* associated with the parent should be copied for the shell process.
* Defaults to TRUE.
*
* SYS_Path (BPTR) -- BCPL pointer to a 'struct PathNode' chain
* which should be used for the shell process.
* Note: this path is used as is and NOT duplicated.
* It will be freed automatically when the shell process exits and
* must be freed manually if the shell process could not be created.
* See; dos.doc/#?CmdPath#?.
*
* SYS_Priority (int8; -128 .. 127) -- Task priority of the shell process.
* Defaults to the same as the parent.
*
* SYS_WindowPtr (APTR) -- Indicate where dos.library requesters should
* be displayed for the shell process (on the Workbench screen,
* a custom screen or not at all).
* Defaults to the same value as the parent process only if it is
* a value of 0 or -1, and if the parent is a process, and the
* SYS_ASynch tag is FALSE, otherwise 0.
*
* EG; A value of 0, indicates requesters should be displayed on
* the default public screen. (Usually WorkBench)
* A value of -1 indicates that no requesters shall be shown.
* Otherwise, a pointer to an open intuition window that all
* requesters should be opened on.
*
* RESULT
* error -- 0 for success.
*
* Depending on the mode used, the result value returned can mean
* different things;
*
* SYS_Asynch,TRUE:
* -1 = indicates System() failure.
* 0 = indicates System() success,
*
* SYS_Asynch,FALSE:
* -1 = indicates System() failure.
* n = non-zero program error.
* 0 = indicates program success. (no error)
*
*
* Note that ONLY with a -1 error code, the caller is responsible for
* deallocating filehandles, paths or other things passed in via tags.
*
* -1 error codes will be returned if DOS could not find the shell,
* or was otherwise unable to start the new shell process.
* Consult IoErr() to find out what actually happened.
*
* NOTES
* From V37+, if an input filehandle is passed, and it's either
* interactive or a "NIL:" filehandle, the pr_ConsolePort of the new
* handler process will be set from that filehandle's port.
*
* This function will also send an ACTION_CHANGE_SIGNAL dospacket to the
* input stream handler upon startup, and again upon returning, restoring
* the sigtask to the parent process. (the same applies to old Execute()).
*
* From 51.77 If an ASYNCH command was started ok, (returncode 0)
* the new process identifier (PID), from the shell process structure field
* process->pr_ProcessID, may be obtained by calling IoErr() immediately
* after this function returns. Note that PID numbers can never be zero.
*
* Some additional NP_xxx tags are allowed to be passed through to the
* internal CreateNewProc() call which starts up the shell handler process,
* this allows for setting up some esoteric things that are not covered
* by the SYS_xxx tags, for the safer functions, SYS_xxx tags have been
* provided as aliases, other ones that could be problematic are not,
* only experienced users should use them and only when understanding
* the possible ramifications.
*
* Some NP_xxx tags cannot be allowed to be passed directly, but instead
* these are generally sent via the startup packet for the shell-handler
* during initialisation, or for a handler process creation attribute,
* or via dedicated SYS_xxx tags, so the following tags will conflict and
* will always be filtered out from any user tags that are supplied;
* NP_Start -- Not to be used.
* NP_Seglist -- SYS_UserShell, SYS_CustomShell determins this.
* NP_FreeSeglist -- Always FALSE.
* NP_Input -- Always 0, Sent via the handler startup packet.
* NP_CloseInput -- Always FALSE.
* NP_Output -- Always 0, Sent via the handler startup packet.
* NP_CloseOutput -- Always FALSE.
* NP_Error -- SYS_Error passes this via the handler process.
* NP_ProgramDir -- Set by the shell handler.
* NP_Cli -- Always TRUE, we definately need one of these.
*
* BUGS
* Before 53.65 launching asynch commands from a workbench started process
* that use clib/newlib wrappers would initialise their own pr_ConsolePort
* and close it when that process ends, if an ASYNCH child shell process
* was started with the SYS_Input tag set to ZERO, it would copy the
* parents pr_ConsolePort which would leave the new ASYNCH child process
* using a dead pr_ConsolePort when the parent process exits.
* So now, specifying a ZERO input stream automatically opens "NIL:"
* no matter what mode is used, this will circumvent the problem.
*
* SEE ALSO
* <dos/dostags.h>, Input(), Output(), ErrorOutput(), DupFileHandle(),
* ProcessScan(), NotifyProcListChange(),
* AddCmdPathNode(), DupCmdPathList(), FreeCmdPathList(),
* SetCurrentCmdPathList(), SearchCmdPathList(), RemoveCmdPathNode().
*
*
************************************************************************
