Another way to create a process with attributes, maybe worse maybe better


Adam Rosenfield noted that "those sure are a lot of hoops you have to jump through to solve this unusual problem" of specifying which handles are inherited by a new process.

Well, first of all, what's so wrong with that? You have to jump through a lot of hoops when you are in an unusual situation. But by definition, most people are not in an unusual situation, so it's an instance of the Pay for Play principle: The simple case should be easy, and it's okay for the complicated case to be hard. (It's usually difficult to make the complicated case easy; that's why it's called the complicated case.)

The complexity mostly comes from managing the general-purpose PROC_THREAD_ATTRIBUTE_LIST, which is used for things other than just controlling inherited handles. It's a generic way of passing up to N additional parameters to Create­Process without having to create 2ᴺ different variations of Create­Process.

The Create­Process­With­Explicit­Handles function was just one of the N special-purpose functions that the PROC_THREAD_ATTRIBUTE_LIST tried to avoid having to create. And the special-purpose function naturally takes the special-purpose case and applies the general solution to it. It's complicated because you are now doing something complicated.

That said, here's one attempt to make it less complicated: By putting all the complicated stuff closer to the complicated function:

typedef struct PROCTHREADATTRIBUTE {
 DWORD_PTR Attribute;
 PVOID lpValue;
 SIZE_T cbSize;
} PROCTHREADATTRIBUTE;

BOOL CreateProcessWithAttributes(
  __in_opt     LPCTSTR lpApplicationName,
  __inout_opt  LPTSTR lpCommandLine,
  __in_opt     LPSECURITY_ATTRIBUTES lpProcessAttributes,
  __in_opt     LPSECURITY_ATTRIBUTES lpThreadAttributes,
  __in         BOOL bInheritHandles,
  __in         DWORD dwCreationFlags,
  __in_opt     LPVOID lpEnvironment,
  __in_opt     LPCTSTR lpCurrentDirectory,
  __in         LPSTARTUPINFO lpStartupInfo,
  __out        LPPROCESS_INFORMATION lpProcessInformation,
    // here is the new stuff
    __in       DWORD cAttributes,
    __in_ecount(cAttributes) const PROCTHREADATTRIBUTE rgAttributes[])
{
 BOOL fSuccess;
 BOOL fInitialized = FALSE;
 SIZE_T size = 0;
 LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList = NULL;

 fSuccess = InitializeProcThreadAttributeList(NULL, cAttributes, 0, &size) ||
            GetLastError() == ERROR_INSUFFICIENT_BUFFER;

 if (fSuccess) {
  lpAttributeList = reinterpret_cast<LPPROC_THREAD_ATTRIBUTE_LIST>
                                (HeapAlloc(GetProcessHeap(), 0, size));
  fSuccess = lpAttributeList != NULL;
 }
 if (fSuccess) {
  fSuccess = InitializeProcThreadAttributeList(lpAttributeList,
                    cAttributes, 0, &size);
 }
 if (fSuccess) {
  fInitialized = TRUE;
  for (DWORD index = 0; index < cAttributes && fSuccess; index++) {
   fSuccess = UpdateProcThreadAttribute(lpAttributeList,
                     0, rgAttributes[index].Attribute,
                     rgAttributes[index].lpValue,
                     rgAttributes[index].cbSize, NULL, NULL);
  }
 }
 if (fSuccess) {
  STARTUPINFOEX info;
  ZeroMemory(&info, sizeof(info));
  info.StartupInfo = *lpStartupInfo;
  info.StartupInfo.cb = sizeof(info);
  info.lpAttributeList = lpAttributeList;
  fSuccess = CreateProcess(lpApplicationName,
                           lpCommandLine,
                           lpProcessAttributes,
                           lpThreadAttributes,
                           bInheritHandles,
                           dwCreationFlags | EXTENDED_STARTUPINFO_PRESENT,
                           lpEnvironment,
                           lpCurrentDirectory,
                           &info.StartupInfo,
                           lpProcessInformation);
 }

 if (fInitialized) DeleteProcThreadAttributeList(lpAttributeList);
 if (lpAttributeList) HeapFree(GetProcessHeap(), 0, lpAttributeList);
 return fSuccess;
}

There, now the complexity is there because you're a generic complex function, so you have no reason to complain.

A caller of this function might go like this:

  HANDLE handles[2] = { handle1, handle2 };
  const PROCTHREADATTRIBUTE attributes[] = {
   {
    PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
    handles,
    sizeof(handles),
   },
  };

  fSuccess = CreateProcessWithAttributes(
                           lpApplicationName,
                           lpCommandLine,
                           lpProcessAttributes,
                           lpThreadAttributes,
                           bInheritHandles,
                           dwCreationFlags,
                           lpEnvironment,
                           lpCurrentDirectory,
                           lpStartupInfo,
                           lpProcessInformation,
                           ARRAYSIZE(attributes),
                           attributes);

Adam hates the "chained success" style and prefers the "goto" style; on the other hand, other people hate gotos. So to be fair, I will choose a coding style that nobody likes. That way everybody is equally unhappy.

BOOL CreateProcessWithAttributes(
  __in_opt     LPCTSTR lpApplicationName,
  __inout_opt  LPTSTR lpCommandLine,
  __in_opt     LPSECURITY_ATTRIBUTES lpProcessAttributes,
  __in_opt     LPSECURITY_ATTRIBUTES lpThreadAttributes,
  __in         BOOL bInheritHandles,
  __in         DWORD dwCreationFlags,
  __in_opt     LPVOID lpEnvironment,
  __in_opt     LPCTSTR lpCurrentDirectory,
  __in         LPSTARTUPINFO lpStartupInfo,
  __out        LPPROCESS_INFORMATION lpProcessInformation,
    // here is the new stuff
    __in       DWORD cAttributes,
    __in_ecount(cAttributes) const PROCTHREADATTRIBUTE rgAttributes[])
{
 BOOL fSuccess = FALSE;
 SIZE_T size = 0;

 if (InitializeProcThreadAttributeList(NULL, cAttributes, 0, &size) ||
     GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
  LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList =
         reinterpret_cast<LPPROC_THREAD_ATTRIBUTE_LIST>
                                (HeapAlloc(GetProcessHeap(), 0, size));
  if (lpAttributeList != NULL) {
   if (InitializeProcThreadAttributeList(lpAttributeList,
                     cAttributes, 0, &size)) {
    DWORD index;
    for (index = 0;
         index < cAttributes &&
         UpdateProcThreadAttribute(lpAttributeList,
                       0, rgAttributes[index].Attribute,
                       rgAttributes[index].lpValue,
                       rgAttributes[index].cbSize, NULL, NULL);
         index++) {
    }
    if (index >= cAttributes) {
     STARTUPINFOEX info;
     ZeroMemory(&info, sizeof(info));
     info.StartupInfo = *lpStartupInfo;
     info.StartupInfo.cb = sizeof(info);
     info.lpAttributeList = lpAttributeList;
     fSuccess = CreateProcess(
                           lpApplicationName,
                           lpCommandLine,
                           lpProcessAttributes,
                           lpThreadAttributes,
                           bInheritHandles,
                           dwCreationFlags | EXTENDED_STARTUPINFO_PRESENT,
                           lpEnvironment,
                           lpCurrentDirectory,
                           &info.StartupInfo,
                           lpProcessInformation);
    }
    DeleteProcThreadAttributeList(lpAttributeList);
   }
   HeapFree(GetProcessHeap(), 0, lpAttributeList);
  }
 }

 return fSuccess;
}

Those who are really adventuresome could try a version of Create­Process­With­Attributes that uses varargs or std::initializer_list.

Comments (17)
  1. Maurits says:

    Does anybody not like return-on-failure?

  2. Mordachai says:

    Generally speaking, I prefer return-on-failure, but am comfortable with any of those three styles (return-on-error, chained success, nested if's); but my favorite is to use C++ and some sort of RAII mechanic (such as lambdas + ScopeGuard to generate the auto-exit-cleanup-on-success in combination with return-on-failure, so that the later failures in the chain still undo the former successes).

  3. Adam Rosenfield says:

    Hate is such a strong word.  But in all fairness, the highly-nested-if-statements style does make me about equally unhappy as it makes fans of the chained-success style.

    @Maurits: I love return-on-failure.  It's like gotos without the gotos.  But in this case, you'd need to make sure to use RAII to avoid resource leaks.

  4. SimonRev says:

    Am I the only one who would have written that as throw-on-failure (combined with RAII)?

  5. Mordachai says:

    Throw on failure is a gray area, in my book.  

    I like it for a lot of things, but in a case like this, the wrapper function is essentially giving the same basic interface as the function its wrapping… so I'd probably choose to do as Raymond did, and put it in a low-level WinAPI extensions library, then, if I wanted throw-on-failure, I'd further wrap the interface in C++ using perhaps std::initialize_list that threw on failure, to make it entirely RAII and C++ friendly.

  6. mikeb says:

    > So to be fair, I will choose a coding style that nobody likes.

    The wisdom of Solomon.

  7. mikeb says:

    I've worked on several projects where return anywhere other than the end of the function was not permitted.  So, yes, there are some people who do not like return-on-failure.

  8. JDP says:

    > I will choose a coding style that nobody likes

    Just like everybody else.

  9. kjh says:

    "Does anybody not like return-on-failure?"

    Yes. There are, believe it or not, still hardcore proponents of SESE (single entry, single exit) out there. I'm not one of them but several of my university CS profs were.

  10. dave says:

    I'm not one of them but several of my university CS profs were.

    Probably because the graph looks nicer :-)

    (I seethe with indignity when someone suggests that the opinion of a code analysis tool is worth more than the opinion of me, a practitioner in the art of designing, implementing, shipping, AND SUPPORTING said code, over what makes code 'better' structured)

  11. Derlin says:

    I once worked on a project with a Single Entry, Single Exit coding standard.  Some of our functions checked for quite a few error conditions, so this made the nested ifs quite frustrating.  I proceeded to write a giant switch statement where the code would break after setting an appropriate error code.  I was accused of violating the spirit of the coding standard, by using a "goto" without the keyword "goto".

  12. Maurits says:

    @Derlin a similar meme is to have a big "do { … } while (FALSE);" around the function, with break-on-failure.

  13. Gabe says:

    Maurits: The advantage of a "switch" is that you can violate the spirit of Single-Entry just as easily as you can violate the spirit of Single-Exit!

  14. Myria says:

    It's interesting how it took until Windows Vista to expose this feature that's been in the native NT API since at least NT 4.0 to the Win32 layer.  Another feature exposed by these attribute lists in Windows Vista is parent process – NT 4.0's NtCreateProcess function already let you specify the parent process.  These may even go as far back as NT 3.1, but I don't have a way to know.

    Here's a feature I'd like to see for Windows 9, that would be an actual new feature: the ability to put a job object as an attribute.  Right now, if you want explicitly to put a child process into a job you create but are not in, you have to CreateProcess as suspended then AssignProcessToJobObject.  This leaves a small window inside which crashing the parent process could result in leaving the child process suspended as a zombie.

    @Maurits [MSFT]: Except that Visual Studio will complain about the conditional expression being constant… >.<  As a result, I tend to use "for (;;)" with a "break;" at the end.

    I wish C++ had Java's syntax of "break label;" and "continue label;" to break or continue a loop other than the current innermost one.

  15. me2 says:

    > I wish C++ had Java's syntax of "break label;" and "continue label;" to break or continue a loop other than the current innermost one.

    it is called "goto"

  16. mario says:

    one question… why did you use reinterpret_cast instead of static_cast in:

    LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList =

            reinterpret_cast<LPPROC_THREAD_ATTRIBUTE_LIST>

                                   (HeapAlloc(GetProcessHeap(), 0, size));

  17. John Doe says:

    The original meaning of "single-entry single-exit" has nothing to do with having only one return instruction in the function. It has to do with having multiple entry points in a function and multiple return addresses, both not made possible with structured languages like plain C/C++.

    programmers.stackexchange.com/…/118793

    So please, call it "one return only".

    In practice, it's stupid doctrine. Imagine forcing one return only in languages that implicitly return from the last executable statement: you'd have to declare a return variable, an extra last statement and assign to that variable instead of letting the value(s) be returned :P blah

    DISCLAIMER: I'm not an unconditional fan of multiple return instructions either, I adopt whatever makes the easiest to read and/or maintain, rarely ever thinking about performance because it doesn't matter for 99.99% of cases, for which profiling will aid better than wild assumptions.

Comments are closed.