CODEWITHSUNDEEP
delphicreateprocess
Steve F
Steve F

Reputation: 1567

Optimal try..finally placement for CreateProcess .. WaitForSingleObject .. CloseHandle calls

I want to call CloseHandle after calls to CreateProcess .. WaitForSingleObject and want to enclose the CloseHandle calls in a try..finally block but not sure where to put the different calls w.r.t. try..finally.

Here is my current code:

var
  p, f, a: String;
  pi: TProcessInformation;
  si: TStartupInfo;
begin
  Log('Starting backup..');

  if (not FileExists(FMYPROG)) then
  begin
    Log('Error: ' + STR_ERRMSG_1);
    MessageDlg(STR_ERRMSG_1, mtError, [mbOK], 0);
    Exit;
  end;

  // start process up
  FillChar(si, SizeOf(si), 0);
  si.cb := SizeOf(si);
  si.dwFlags := STARTF_USESHOWWINDOW;
  si.wShowWindow := SW_NORMAL;

  f := IncludeTrailingPathDelimiter(FBAKFILEPATH) + 'output.bak';
  p := '/changesonly "' + f + '"';

  try   // is this the optimal placement for this line? or should it be after CreateProcess? 
    if CreateProcess(PChar(FMYPROG), PChar(p), nil, nil, False,
      CREATE_NEW_PROCESS_GROUP + NORMAL_PRIORITY_CLASS, nil, PChar(ExtractFilePath(FMYPROG)), si, pi) then
      WaitForSingleObject(pi.hProcess, INFINITE)
    else
      RaiseLastOSError;

  finally
    CloseHandle(pi.hProcess);
    CloseHandle(pi.hThread);
  end;

Suggestions and critique leading to Delphi enlightenment sought. Thank you.

Upvotes: 2

Views: 616

Answers (1)

David Heffernan
David Heffernan

Reputation: 612904

Only call CloseHandle if CreateProcess succeeds. Therefore it goes like this:

if CreateProcess(...) then
  try
    ....
  finally
    // calls to CloseHandle
  end
else
  RaiseLastOSError;

Or if you prefer to deal with the error cases up-front:

if not CreateProcess(...) then
  RaiseLastOSError;
try
  ....
finally
  // calls to CloseHandle
end

This is semantically identical because you know that RaiseLastOSError will raise an exception.

Or as I prefer:

Win32Check(CreateProcess(...));
try 
  ....
finally
  // calls to CloseHandle
end;

The Win32Check convenience function simply encapsulates the logic

if not Succeeded then
  RaiseLastOSError;

Upvotes: 4

Related Questions