Is there any problem to using this code in a Thread ? (Delphi)

Question

i use this code in a thread (through Indy Onexecute event) . is there any problem ?

function TFrmMain.ShellExecute_AndWait(FileName, Params: string): bool;
var
  exInfo: TShellExecuteInfo;
  Ph: DWORD;
begin
  FillChar(exInfo, SizeOf(exInfo), 0);
  with exInfo do
  begin
    cbSize := SizeOf(exInfo);
    fMask := SEE_MASK_NOCLOSEPROCESS or SEE_MASK_FLAG_DDEWAIT;
    Wnd := GetActiveWindow();
    exInfo.lpVerb := 'open';
    exInfo.lpParameters := PChar(Params);
    lpFile := PChar(FileName);
    nShow := SW_NORMAL;
  end;
  if ShellExecuteEx(@exInfo) then
    Ph := exInfo.hProcess
  else
  begin
    Result := true;
    exit;
  end;
  while WaitForSingleObject(exInfo.hProcess, 50) <> WAIT_OBJECT_0 do
  begin

  end;
  CloseHandle(Ph);
  Result := true;
end;

Answer 1

MSDN has this advice:

Because ShellExecuteEx can delegate execution to Shell extensions (data sources, context menu handlers, verb implementations) that are activated using Component Object Model (COM), COM should be initialized before ShellExecuteEx is called. Some Shell extensions require the COM single-threaded apartment (STA) type. In that case, COM should be initialized as shown here:

CoInitializeEx(NULL, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE)

There are instances where ShellExecuteEx does not use one of these types of Shell extension and those instances would not require COM to be initialized at all. Nonetheless, it is good practice to always initalize COM before using this function.

(In Delphi, you'd of course replace the first parameter with nil and use or for the bitwise operation.)

Raymond Chen recently wrote about the consequences of getting this wrong. The specific example was that the function might fail with the Error_Access_Denied error code.

That's the only potential multithreading issue I see in your code. Below are further things that occurred to me when I read your code, although they have nothing to do with multithreading (and not even much to do with Indy).

---

You have a peculiar way of waiting for the program to stop running. You repeatedly wait for 50 milliseconds at a time, but if the process isn't finished yet, you do nothing but wait again. Describe your intention more accurately by specifying Infinite for the timeout.

---

The function always returns True. If there's no useful return value, then you should just make it a procedure so there's no return value at all. Don't confuse the caller with useless information. If you're going to keep it as a function, then use the Delphi native type Boolean instead of the Windows compatibility type Bool for the return type.

---

I'm a little wary about the idea of a server executing user-interactive programs upon receipt of network messages.

---

Notice when MSDN says you might not get a process handle. There are cases when ShellExecuteEx can service your request without creating a new process, so you'll have nothing to wait on.

The user might end up using the program awhile, and your server will be stuck waiting all that time. I wonder whether it really needs to wait at all. Is the client going to be waiting for a response from the server, too?