4

Using: Delphi XE2; Windows 32-bit VCL application

From within my Delphi application, I need to call an application using ShellExecute and wait until it finishes before proceeding.

I see many examples here on SO of ShellExecute with MsgWaitForMultipleObjects but can't know which one is the best because they are mostly doing what is not recommended ie. also using Application.ProcessMessages which is not recommended by many.

I see an answer by NFX here in this post which does not use Application.ProcessMessages, but am not sure if it is correct or optimum, and hence this question.

Would be glad if you could provide a good quality code sample.

TIA for any answers.

Community
  • 1
  • 1
Steve F
  • 1,527
  • 1
  • 29
  • 55

3 Answers3

4

ShellExecuteEx() and CreateProcess() both return a HANDLE that you can wait on. The HANDLE is signaled when the spawned process exits.

If you have to pump a message queue while waiting, use MsgWaitForMultipleObjects() to detect when a new message is waiting to be processed. Otherwise, you can use WaitForSingleObject() instead.

NFX's answer does not use Application.ProcessMessages() but it still pumps messages nonetheless, so the root issue remains. If you are doing the waiting in the main thread, you cannot avoid that unless you do not mind presenting an unresponsive UI to your users (or the OS). You could alternatively do the wait in a worker thread instead, then you don't need to pump messages while waiting, and your UI will not be blocked. You can disable your UI or display a status UI while waiting, if you want.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
3

I use these functions to execute a child process asynchronously and have it call back when the process terminates. It works by creating a thread that waits until the process terminates and then calls back to the main program thread via the event method given. Beware, that your program continues to run while the child process is running, so you'll need some form of logic to prevent an infinite occurence of spawning child processes.

UNIT SpawnFuncs;

INTERFACE

{$IF CompilerVersion >= 20 }
  {$DEFINE ANONYMOUS_METHODS }
{$ELSE }
  {$UNDEF ANONYMOUS_METHODS }
{$ENDIF }

TYPE
  TSpawnAction  = (saStarted,saEnded);
  TSpawnArgs    = RECORD
                    Action      : TSpawnAction;
                    FileName    : String;
                    PROCEDURE   Initialize(Act : TSpawnAction ; CONST FN : String); INLINE;
                    CLASS FUNCTION Create(Act : TSpawnAction ; CONST FN : String) : TSpawnArgs; static;
                  END;
  {$IFDEF ANONYMOUS_METHODS }
    TSpawnEvent = REFERENCE TO PROCEDURE(Sender : TObject ; CONST Args : TSpawnArgs);
  {$ELSE }
    TSpawnEvent = PROCEDURE(Sender : TObject ; CONST Args : TSpawnArgs) OF OBJECT;
  {$ENDIF }

FUNCTION ShellExec(CONST FileName,Tail : String ; Event : TSpawnEvent = NIL ; Sender : TObject = NIL) : BOOLEAN; OVERLOAD;
FUNCTION ShellExec(CONST FileName : String ; Event : TSpawnEvent = NIL ; Sender : TObject = NIL) : BOOLEAN; OVERLOAD;
FUNCTION ShellExec(CONST FileName : String ; VAR EndedFlag : BOOLEAN) : BOOLEAN; OVERLOAD;
FUNCTION ShellExec(CONST FileName,Tail : String ; VAR EndedFlag : BOOLEAN) : BOOLEAN; OVERLOAD;

PROCEDURE ShellExecExcept(CONST FileName : String ; Event : TSpawnEvent = NIL ; Sender : TObject = NIL); OVERLOAD:
PROCEDURE ShellExecExcept(CONST FileName,Tail : String ; Event : TSpawnEvent = NIL ; Sender : TObject = NIL); OVERLOAD;
PROCEDURE ShellExecExcept(CONST FileName : String ; VAR EndedFlag : BOOLEAN); OVERLOAD;
PROCEDURE ShellExecExcept(CONST FileName,Tail : String ; VAR EndedFlag : BOOLEAN); OVERLOAD;

IMPLEMENTATION

USES Windows,SysUtils,Classes,ShellApi;

TYPE
  TWaitThread   = CLASS(TThread)
                    CONSTRUCTOR Create(CONST FileName : String ; ProcessHandle : THandle ; Event : TSpawnEvent ; Sender : TObject); REINTRODUCE; OVERLOAD;
                    CONSTRUCTOR Create(CONST FileName : String ; ProcessHandle : THandle ; EndedFlag : PBoolean); OVERLOAD;
                    PROCEDURE   Execute; OVERRIDE;
                    PROCEDURE   DoEvent(Action : TSpawnAction);
                  PRIVATE
                    Handle      : THandle;
                    Event       : TSpawnEvent;
                    EndedFlag   : PBoolean;
                    FN          : String;
                    Sender      : TObject;
                    {$IFNDEF ANONYMOUS_METHODS }
                      Args      : TSpawnArgs;
                      PROCEDURE RunEvent;
                    {$ENDIF }
                  END;

CONSTRUCTOR TWaitThread.Create(CONST FileName : String ; ProcessHandle : THandle ; Event : TSpawnEvent ; Sender : TObject);
  BEGIN
    INHERITED Create(TRUE);
    Handle:=ProcessHandle; Self.Event:=Event; FN:=FileName; Self.Sender:=Sender; FreeOnTerminate:=TRUE;
    Resume
  END;

{$IFNDEF ANONYMOUS_METHODS }
PROCEDURE TWaitThread.RunEvent;
  BEGIN
    Event(Sender,Args)
  END;
{$ENDIF }

CONSTRUCTOR TWaitThread.Create(CONST FileName : String ; ProcessHandle : THandle ; EndedFlag : PBoolean);
  BEGIN
    INHERITED Create(TRUE);
    Handle:=ProcessHandle; EndedFlag^:=FALSE; Self.EndedFlag:=EndedFlag; FreeOnTerminate:=TRUE;
    Resume
  END;

PROCEDURE TWaitThread.DoEvent(Action : TSpawnAction);
  BEGIN
    IF Assigned(EndedFlag) THEN
      EndedFlag^:=(Action=saEnded)
    ELSE BEGIN
      {$IFDEF ANONYMOUS_METHODS }
        Synchronize(PROCEDURE BEGIN Event(Sender,TSpawnArgs.Create(Action,FN)) END)
      {$ELSE }
        Args:=TSpawnArgs.Create(Action,FN);
        Synchronize(RunEvent)
      {$ENDIF }
    END
  END;

PROCEDURE TWaitThread.Execute;
  BEGIN
    DoEvent(saStarted);
    WaitForSingleObject(Handle,INFINITE);
    CloseHandle(Handle);
    DoEvent(saEnded)
  END;

FUNCTION ShellExec(CONST FileName,Tail : String ; Event : TSpawnEvent ; Sender : TObject ; EndedFlag : PBoolean) : BOOLEAN; OVERLOAD;
  VAR
    Info  : TShellExecuteInfo;
    PTail : PChar;

  BEGIN
    ASSERT(NOT (Assigned(Event) AND Assigned(EndedFlag)),'ShellExec called with both Event and EndedFlag!');
    IF Tail='' THEN PTail:=NIL ELSE PTail:=PChar(Tail);
    FillChar(Info,SizeOf(TShellExecuteInfo),0);
    Info.cbSize:=SizeOf(TShellExecuteInfo);
    Info.fMask:=SEE_MASK_FLAG_NO_UI;
    Info.lpFile:=PChar(FileName);
    Info.lpParameters:=PTail;
    Info.nShow:=SW_SHOW;
    IF NOT (Assigned(Event) OR Assigned(EndedFlag)) THEN
      Result:=ShellExecuteEx(@Info)
    ELSE BEGIN
      Info.fMask:=Info.fMask OR SEE_MASK_NOCLOSEPROCESS;
      Result:=ShellExecuteEx(@Info) AND (Info.hProcess>0);
      IF Result THEN
        IF Assigned(Event) THEN
          TWaitThread.Create(FileName,Info.hProcess,Event,Sender)
        ELSE
          TWaitThread.Create(FileName,Info.hProcess,EndedFlag)
    END
  END;

FUNCTION ShellExec(CONST FileName,Tail : String ; Event : TSpawnEvent = NIL ; Sender : TObject = NIL) : BOOLEAN;
  BEGIN
    Result:=ShellExec(FileName,Tail,Event,Sender,NIL)
  END;

FUNCTION ShellExec(CONST FileName : String ; Event : TSpawnEvent = NIL ; Sender : TObject = NIL) : BOOLEAN;
  BEGIN
    Result:=ShellExec(FileName,'',Event,Sender)
  END;

FUNCTION ShellExec(CONST FileName,Tail : String ; VAR EndedFlag : BOOLEAN) : BOOLEAN;
  BEGIN
    Result:=ShellExec(FileName,Tail,NIL,NIL,@EndedFlag)
  END;

FUNCTION ShellExec(CONST FileName : String ; VAR EndedFlag : BOOLEAN) : BOOLEAN;
  BEGIN
    Result:=ShellExec(FileName,'',EndedFlag)
  END;

PROCEDURE ShellExecExcept(CONST FileName : String ; Event : TSpawnEvent = NIL ; Sender : TObject = NIL);
  BEGIN
    IF NOT ShellExec(FileName,Event,Sender) THEN RaiseLastOSError
  END;

PROCEDURE ShellExecExcept(CONST FileName,Tail : String ; Event : TSpawnEvent = NIL ; Sender : TObject = NIL);
  BEGIN
    IF NOT ShellExec(FileName,Tail,Event,Sender) THEN RaiseLastOSError
  END;

PROCEDURE ShellExecExcept(CONST FileName : String ; VAR EndedFlag : BOOLEAN);
  BEGIN
    IF NOT ShellExec(FileName,EndedFlag) THEN RaiseLastOSError
  END;

PROCEDURE ShellExecExcept(CONST FileName,Tail : String ; VAR EndedFlag : BOOLEAN);
  BEGIN
    IF NOT ShellExec(FileName,Tail,EndedFlag) THEN RaiseLastOSError
  END;

{ TSpawnArgs }

CLASS FUNCTION TSpawnArgs.Create(Act : TSpawnAction ; CONST FN : String) : TSpawnArgs;
  BEGIN
    Result.Initialize(Act,FN)
  END;

PROCEDURE TSpawnArgs.Initialize(Act : TSpawnAction ; CONST FN : String);
  BEGIN
    Action:=Act; FileName:=FN
  END;

END.

Use it as follows:

USES SpawnFuncs;

ShellExec(ProgramToRun,CommandLineArgs,Event,Sender)

or

ShellExec(ProgramToRunOrFileToOpen,Event,Sender)

where

ProgramToRun = Name of program to run
ProgramToRunOrFileToOpen = Program to run, or file to open (f.ex. a .TXT file)
CommandLineArgs = Command line parameters to pass to the program
Event = The (perhaps anonymous) method to run upon start and termination of program
Sender = The Sender parameter to pass to the method

Or, if you are simply interested in knowing when the child process has terminated, there are two simplified versions that accept a BOOLEAN variable that will be set to TRUE as soon as the child program terminates. You don't need to set it to FALSE first, as it will be done automatically:

ShellExec(ProgramToRun,ChildProcessEnded);

If you don't supply an event handler or BOOLEAN variable, the ShellExec procedure simply runs/opens the file given and performs no callback.

If you don't supply a Sender, the Sender parameter will be undefined in the event handler.

The event handler must be a method (anonymous or otherwise) with the following signature:

PROCEDURE SpawnEvent(Sender : TObject ; CONST Args : TSpawnArgs);

where Args contains the following fields:

Action = either saStarted or saEnded
FileName = the name of the file that passed to ShellExec

If you prefer to use SEH (Structured Exception Handling) instead of error return values, you can use the ShellExecExcept PROCEDUREs instead of the ShellExec FUNCTIONs. These will raise an OS Error in case the execute request failed.

HeartWare
  • 7,464
  • 2
  • 26
  • 30
  • Because I haven't had the need for it in my usage so far... But you have a point. Code updated to make the PROCEDUREs FUNCTIONs instead to allow caller to be aware of the result... – HeartWare Apr 12 '14 at 19:19
  • 1
    @HeartWare: Thanks for posting this! Much appreciated. – Steve F Apr 12 '14 at 19:29
  • @SteveF: Well, you deleted your question on how to prevent the "infinite occurence of spawning child processes", but here's my answer anyway: It depends entirely on what you want your main application to do while the external program is running. Do you want it to be complete non-responsive (ie. allow no interaction from the user at all, and perhaps give the (in)famous "Process not responding" if the user tries)? Or do you want to allow the user to perform other tasks in your program while the external program is running, except run a new external program? – HeartWare Apr 12 '14 at 19:30
  • @HeartWare: I'd like it to wait and not respond to any other user input. – Steve F Apr 12 '14 at 19:39
  • In that case, you'll need a member variable (f.ex. ChildProcessEnded) in your form that you start by setting to FALSE before starting the child process, and in the call-back you set it to TRUE once you are called with saEnded. Your child process execution should then be IF ShellExec(...) THEN WHILE NOT ChildProcessEnded DO Sleep(100); This way, your main application will be completely non-responsive as long as the child process is running. This, however, is not recommended behaviour, as your application will appear to have crashed while the child process is running. – HeartWare Apr 12 '14 at 19:42
  • I'm unable to pass SpawnEvent as a parameter. Its defined as a procedure of my main form. – Steve F Apr 12 '14 at 20:04
  • Forgot to add CONST specifier to the Event type declaration. Use the updated UNIT instead of the previous one... – HeartWare Apr 12 '14 at 20:11
  • I'm trying the code with a simple application ie. Window's Calc.Exe. The problem I'm having is that after closing Calc, the calling program remains frozen and unresponsive. I need to end it via Task Manager. I'm running on Windows 8.1. – Steve F Apr 12 '14 at 20:33
  • Then you're not setting the member variable ChildProcessEnded to TRUE in your event method, when it gets called with saEnded, or your wait loop is incorrectly coded... – HeartWare Apr 12 '14 at 20:36
  • procedure TfrmMain.FormShow(Sender: TObject); begin FChildProcessEnded := False; end; procedure TfrmMain.SpawnEvent(Sender: TObject; const Args: TSpawnArgs); begin if Args.Action = saEnded then FChildProcessEnded := True; end; if ShellExec(f, SpawnEvent, Sender) then while not FChildProcessEnded do Sleep(100); – Steve F Apr 12 '14 at 20:39
  • I am setting it to True correctly in the event method, and the wait loop seems okay. So why is the calling program freezing? – Steve F Apr 12 '14 at 20:46
  • I ran it in the debugger and it does not seem to be entering into SpawnEvent at all.. – Steve F Apr 12 '14 at 20:59
  • Can you post how you are calling ShellExec with the SpawnEvent parameter? and your declaration of SpawnEvent in your form/unit? – Steve F Apr 12 '14 at 21:00
  • Because the callback to the main thread is being done via messages (didn't take that into account, when I suggested the Sleep loop), so unless the main thread is processing messages, the callback is not working. I have made some changes to the UNIT so that you can now call it with a BOOLEAN variable that will be FALSE while the program is running, and will be set to TRUE when the program is terminated. No need for a callback event in this case... – HeartWare Apr 12 '14 at 21:06
  • Seems to work now, w/o using SpawnEvent. Calling it like this: ShellExec(f, FChildProcessEnded); while not FChildProcessEnded do Sleep(100); – Steve F Apr 12 '14 at 21:18
  • Waiting for the `Handle` event for `INFINITE` time will block the `TWaitThread` to terminate when closing your application unless the executed process terminates, which might not always be wanted. In other words, that thread will prevent your application to die unless you close the executed one. You should add some event signalled when that thread is terminated and wait for both with `WaitForMultipleObjects`. – TLama Apr 12 '14 at 21:58
  • Sleep pretty much sucks here. – David Heffernan Apr 12 '14 at 22:11
  • @TLama: I have just tried it, and the application terminates without problems when I click the "X" close button, even though the child process is still running... I don't get called in the callback, so the thread is forcibly terminated. – HeartWare Apr 12 '14 at 22:16
  • @DavidHeffernan: Agree that using Sleep isn't nice, unless in very specific circumstances, but this is what the OP wanted, so... :-) As you may have read, I also recommended against this practice earlier in the comments. – HeartWare Apr 12 '14 at 22:17
  • @HeartWare, then maybe the system releases the executed process handle and the handle gets the undefined state. I don't have Delphi by hand to examine that part. But I'm surprised a little bit. – TLama Apr 12 '14 at 22:20
  • @TLama: If that was the case, wouldn't I get called in the call-back? It seems like the thread is forcibly terminated, ie. it never returns from the WaitForSingleObject call. – HeartWare Apr 12 '14 at 22:22
  • @TLama: Yup - just tried setting a breakpoint right after the WaitForSingleObject call, and the breakpoint is never hit. – HeartWare Apr 12 '14 at 22:24
  • (Using Delphi XE5 and Win 7 Pro) – HeartWare Apr 12 '14 at 22:26
  • @tlama depending on how the app is terminated it is quite capable of dropping all waits and quitting forcibly. ExitProcess will do that. When a process terminates all of its handles are closed. – David Heffernan Apr 12 '14 at 22:32
  • @David, as I said, I don't have Delphi by hand, but how Delphi decides when to force the application to terminate ? And even then, if the handle would be closed, fine, the wait would fail, but the next line would execute, doesn't it ? – TLama Apr 12 '14 at 22:34
  • @tlama not very pretty I agree. I'd do what you suggest. But the system tidies all resources up at close down anyway. – David Heffernan Apr 12 '14 at 22:38
  • @all: So does everyone agree that this is good quality code worthy of production use? – Steve F Apr 12 '14 at 23:04
  • @steve I would not agree – David Heffernan Apr 13 '14 at 06:23
  • @DavidHeffernan: And why is that? – HeartWare Apr 13 '14 at 07:24
  • Hard to read. Poor error checking. Use of ShellExecute which doesn't report errors properly. Use of ShellExecuteEx rather than CreateProcess. – David Heffernan Apr 13 '14 at 07:34
  • "Hard to read" is very subjective. Use of ShellExecute allows you to execute .TXT files or other non-executable files via default application, which CreateProcess doesn't, so this is by design. What kind of error checking are you missing? – HeartWare Apr 13 '14 at 07:38
  • there error checking is just succeed or fail. Nothing more. You need to use ShellExecuteEx and RaiseLastOSError if it fails. Question does not asks about waiting which demands CreateProcess. – David Heffernan Apr 13 '14 at 14:50
  • Or the user should call GetLastError if the function returns FALSE. The error checking is there... It's a matter of taste if you want an exception or a return value - I most often prefer the latter, so that's how I design it. I don't understand your "demands CreateProcess". There's nothing that *demands* CreateProcess. Almost everything (if not actually *everything* relating to executing an .EXE file) that can be done via CreateProcess (within the needs set out for the function) can be done via ShellExecute, but ShellExecute can do so much more, so why use an inferior system call? – HeartWare Apr 13 '14 at 18:02
  • Also, I don't understand "Question does not asks about waiting", as it most assuredly does (and this does not "demand" CreateProcess, as you can see from the code, which is perfectly capable of waiting till exit even though it uses ShellExecute). – HeartWare Apr 13 '14 at 18:05
  • You have to call GetLastError immediately. You don't. Plus you call ShellExecute. I typed wrong. Question does ask about waiting. That requires starting new process rather than relying on shell association. – David Heffernan Apr 13 '14 at 21:24
  • If you don't use @myname I don't get notified. – David Heffernan Apr 13 '14 at 21:24
  • @DavidHeffernan Whether I or the caller calls GetLastError doesn't matter. A simple IF NOT ShellExecute(...) THEN RaiseLastOSError; in the user's code will convert the return value into an OS Exception if that's what the caller wants... – HeartWare Apr 14 '14 at 04:28
  • @DavidHeffernan Why does waiting "require starting new process rather than relying on shell association"??? Waiting works fine using .EXE files passed to ShellExecute (which is all that CreateFile can be passed). ShellExecute *also* works fine for most files with a shell association. So which .EXE file works for waiting via CreateFile but not via ShellExecute? Give me an example, please... – HeartWare Apr 14 '14 at 04:32
  • @DavidHeffernan From ShellExecuteEx documentation: "Even if fMask is set to SEE_MASK_NOCLOSEPROCESS, hProcess will be NULL if no process was launched. For example, if a document to be launched is a URL and an instance of Internet Explorer is already running, it will display the document. No new process is launched, and hProcess will be NULL. ShellExecuteEx does not always return an hProcess, even if a process is launched as the result of the call. For example, an hProcess does not return when you use SEE_MASK_INVOKEIDLIST to invoke IContextMenu." Which of these cases would work using CreateP.? – HeartWare Apr 14 '14 at 05:46
  • @heartware none. But that's not the point. SEE is for when you want the shell to do some work. If you already know the exe then CP is the api to call. Unless you need the runas verb. You should never ever use SE. No useful error reporting. RaiseLastOSError is right choice here. – David Heffernan Apr 14 '14 at 05:47
  • You can't send a URL to CreateProcess, and last I checked, neither could you invoke IContextMenu via CreateProcess. – HeartWare Apr 14 '14 at 05:47
  • @DavidHeffernan That's *excactly* the point. You say that I should use CreateProcess. But ShellExecute works just as well in all cases that CreateProcess does, and *also* works well in most other cases, where you want to execute an associated file. So if I replace ShellExecute with CreateProcess, what will I gain? Which cases will work better than before? I know what I will lose, but what will I gain? – HeartWare Apr 14 '14 at 05:49
  • CP is not a subset of SEE. Process creation flags. Redirection. Desktop control. Suspended process creation etc. Even without that why invoke SEE to call CP? May as well call CP directly. – David Heffernan Apr 14 '14 at 05:52
  • Sorry - I don't buy an argument that just says "Because" or "Because I say so" or "Because that's the way it's *supposed* to be done". If my current code works better than the "supposed to" code does, I won't replace it. If there's nothing to gain, and something to lose by doing it the way it's "supposed to", then I don't mind not doing it the it's "supposed to". – HeartWare Apr 14 '14 at 05:52
  • Also the readability is an issue. You might have an unusual style but on SO you should use common formatting. I know I change my style here to fit in. It's obvious why. – David Heffernan Apr 14 '14 at 05:53
  • You did not read what I wrote it seems. I gave plenty of reasons. – David Heffernan Apr 14 '14 at 05:54
  • Okay - how would you use CP to execute "http://www.google.com"? Or "C:\Windows\Web\Wallpaper\Nature\img1.jpg"? I have other functions in case I want to capture the output of executed files (which *does* use CP). But the functions used in the code above are not meant to capture output, nor create suspended processes, so why use the inferior CP when SEE works just as well - or even better in that it can "execute" associated files? – HeartWare Apr 14 '14 at 05:55
  • You also willfully ignored criticism of use of SE. It's still there. You have to stop using SE. – David Heffernan Apr 14 '14 at 05:56
  • SEE is the function for those cases. – David Heffernan Apr 14 '14 at 05:56
  • I always read what you write (to me). If you don't get your point across, it's your writing that's at fault (at least that's what I learned at a course in communications: It's the sender's responsibility that the message is understood. If it isn't understood - barring obvious misinterpretation - then the message wasn't clear to begin with). – HeartWare Apr 14 '14 at 05:57
  • Why do I "have to stop using SE" (which I don't BTW, in the cases where I need hProcess)? – HeartWare Apr 14 '14 at 05:58
  • SE does not report errors properly. It is retained for back compat. SEE reports errors correctly. – David Heffernan Apr 14 '14 at 06:00
  • My philosophy here is simple. If you know the executable then none of the shell related functionality of SEE will be used. So CP offers more. – David Heffernan Apr 14 '14 at 06:03
  • Okay, now we may be going somewhere. In which cases does SE "not report errors properly"? If a satisfactory reply can be given to this question, I will change my use of SE to SEE, even in the cases where I don't need the hProcess... – HeartWare Apr 14 '14 at 06:03
  • But your philosophy is irrelevant in the cases where this function is to be used. The functions given here is specifically, and intentionally, written so that you can call both an executable and an association, and for the task these functions are to solve, what benefits would I get from using CP instead of SEE? Like I said, I know what benefits I would *lose*, but which would I gain? – HeartWare Apr 14 '14 at 06:05
  • Your use of this code is not pertinent to the question. As I understand the question, asker knows the executable. – David Heffernan Apr 14 '14 at 06:10
  • SE error reporting: http://blogs.msdn.com/b/oldnewthing/archive/2012/10/18/10360604.aspx – David Heffernan Apr 14 '14 at 06:11
  • And the supplied code works fine in the case where the caller knows the executable - but *also* works fine in the cases where the caller doesn't. So once again: What are the benefits of using CP instead of SEE, even when the user knows the executable (within the confines of the needs of the function)? Which things does using CP give the caller that using SEE doesn't? – HeartWare Apr 14 '14 at 06:15
  • SE: Thank you for the article, but it doesn't alter anything. Using SEE instead of SE in the place in my code won't alter anything. In both cases the precise error code is available via GetLastError, which is what RaiseLastOSError uses, so changing my SE code to SEE code, won't bring any benefits within the confines of what the function is meant to do. – HeartWare Apr 14 '14 at 06:17
  • SE does not use SetLastError. You do need to use SEE if you want detailed error reporting. CP allows all the extras that I said before. Redirection. Creation flags etc. – David Heffernan Apr 14 '14 at 06:25
  • @DavidHeffernan: SE *does* use SetLastError (confirmed by me clearing it before I call ShellExecute). And what extras does CP allow that is pertinent to the actions this function is to perform (which doesn't include capturing output, special process creation flags, desktop control, suspended process creation etc.). Given the input parameters to the function is FileName, optional command line, and an event to call when the process terminates, which benefits would be gained by using CP instead of SEE? Give me an example, please... – HeartWare Apr 14 '14 at 06:35
  • If SE could be relied upon to report errors using SetLastError the docs would say so. – David Heffernan Apr 14 '14 at 06:41
  • Anyway, as for CP vs SEE, if both give same functionality either can be used. I would opt for CP personally for the philosophical reasons outlined above. Why add an extra layer? But either way works that is clear. – David Heffernan Apr 14 '14 at 06:50
  • 1
    @DavidHeffernan: Okay, I'm now using SEE in all cases, just to satisfy you :-). Now what arguments are left against this being "good quality code worthy of production use". Not "using SE". Not "not checking for errors". So what's left? – HeartWare Apr 14 '14 at 06:53
  • Now it's just my personal taste. I don't like your capitalisation and layout, I don't like the error handling (prefer RaiseLastOSError), and I prefer CP. But those are clearly my personal problems. – David Heffernan Apr 14 '14 at 06:55
  • Anon methods were coincidentally introduced in same version as Unicode. But the test really should be against VER200. – David Heffernan Apr 14 '14 at 07:01
  • @DavidHeffernan: Now, just for you :-), I have implemented the code I suggested earlier, so that the caller can choose between error return value or exception. If the latter is wanted, just use the PROCEDUREs ShellExecExcept instead of the FUNCTIONs ShellExec. See - best of both worlds :-). I have also changed the UNICODE into a compiler version test instead... – HeartWare Apr 14 '14 at 07:04
  • Cool. I've enjoyed this conversation!! – David Heffernan Apr 14 '14 at 07:06
  • So, in other words, the *compiled code* is now "good quality code worthy of production use", for the usage that is needed by the OP? – HeartWare Apr 14 '14 at 07:11
  • I personally wouldn't use it in my production code. But that's just me. ;-) – David Heffernan Apr 14 '14 at 07:12
  • Well, I take it that that's the best endorsement you can give me (not that you wouldn't use it in *your* production code, but that it is the only objection you have left) :-). – HeartWare Apr 14 '14 at 07:15
  • FWIW In the past I've used wrappers around CP and SEE. But not any more. I find it more productive and flexible to call them directly. In the case of SEE I have helpers to populate instances of the `SHELLEXECUTEINFO` struct. My experience with large wrapper layers is that they just obscure the detail. I think when I started using wrappers it was largely because I did not understand the detail. Now that I do I prefer to go straight to the root API. Obviously that's a quite personal preference of mine. – David Heffernan Apr 14 '14 at 07:39
2
function StartProcess(Exename: string; CmdLineArgs: string = ''; ShowWindow: boolean = True;
  WaitForFinish: boolean = False): integer;
var
  StartInfo: TStartupInfo;
  ProcInfo: TProcessInformation;
  CreateOK: boolean;
begin
  // Simple wrapper for the CreateProcess command
  // returns the process id of the started process.
  FillChar(StartInfo, SizeOf(TStartupInfo), #0);
  FillChar(ProcInfo, SizeOf(TProcessInformation), #0);
  StartInfo.cb := SizeOf(TStartupInfo);

  if not(ShowWindow) then
  begin
    StartInfo.dwFlags := STARTF_USESHOWWINDOW;
    StartInfo.wShowWindow := SW_HIDE;
  end;

  CreateOK := CreateProcess(nil, PChar(Exename + ' ' + CmdLineArgs), nil, nil, False,
    CREATE_NEW_PROCESS_GROUP + NORMAL_PRIORITY_CLASS, nil, nil, StartInfo, ProcInfo);

  Result := ProcInfo.dwProcessId;

  if CreateOK then
  begin
    // may or may not be needed. Usually wait for child processes
    if WaitForFinish then
      WaitForSingleObject(ProcInfo.hProcess, Infinite);
  end
  else
  begin
    // ShowMessage('Unable to run '+ProgramName);
    SysErrorMessage(GetLastError());
  end;

  // close process & thread handles
  CloseHandle(ProcInfo.hProcess);
  CloseHandle(ProcInfo.hThread);
end;
Steve F
  • 1,527
  • 1
  • 29
  • 55
  • 1
    Please don't post code copied from somewhere else without explanation. Zeroising `ProcInfo` is needless. I think I'd specify both first and second args to `CreateProcess`. I'd put a try/finally in to protect the handles. Run this on your main thread and it will block and not pump queue. Do you want that. – David Heffernan Apr 12 '14 at 19:12
  • @DavidHeffernan: Why do you think I post my original question here on SO - so that I can get answers, but all you do is redirect me to MSDN. I started programming with Turbo C and know both C and C++ programming. But I still can't understand Windows API as documented in MSDN. Thats why I post here on SO in hope of getting answers. I don't want to refer to MSDN. Its silly, as is anyones mindless suggestions to 'refer' to MSDN when a programmer is asking a question on SO. I'm not a Visual C++ programmer and never ever want to be one. – Steve F Apr 12 '14 at 19:16
  • I find this attitude depressing. I don't code Windows in C or C++ routinely. Does not stop me being able to read the docs. Refusing to read the docs for your platform gives you a huge handicap. Can't imagine why you would choose to do that. – David Heffernan Apr 12 '14 at 22:16
  • 1
    Its because I always used to read the docs on MSDN and find it hard to understand especially Windows API stuff. Then to translate that C++ code and conventions into Delphi is a whole another story that involves trial and error and still not knowing whether you got it right until an 'expert' sees your translated Delphi code and makes comments. So while the MSDN documentation for Windows API is a useful guide for Delphi programmers attempting to implement API functions, its can also be counter-productive (for some?). – Steve F Apr 12 '14 at 23:03