[Interest] Enforce only stdin/out/err inherited with QProcess

[christoph at cullmann.io]

On 2024-09-25 20:15, christoph at cullmann.io wrote:
> On 2024-09-25 19:48, Thiago Macieira wrote:
>> On Wednesday 25 September 2024 08:36:47 GMT-7 christoph at cullmann.io 
>> wrote:
>>> > Since you want no handles but the standard three, would setting
>>> >
>>> >   args->inheritHandles = false
>>> >
>>> > suffice?
>>> 
>>> I tried that but for me that fails, I have no handles and unlike on
>>> Linux
>>> or Mac with the other code path I can no longer read any output from 
>>> the
>>> QProcess.
>>> 
>>> I did read a bit the Windows docs and the Qt code and its seems
>>> inheritHandles = TRUE
>>> is required if you use the STARTF_USESTDHANDLES flag in the startup 
>>> info
>>> like Qt does.
>> 
>> Ah, ok.
>> 
>> So it looks like doing what you tried to do by understanding Raymond 
>> Chen's
>> blog is the way to go... except it didn't work. Aside from the fact 
>> that you
>> didn't clean the thread's handle state or at least didn't include it 
>> in the
>> code you pasted, I see no reason it shouldn't work.
> 
> Yeah, that cleanup code would be after the start().
> 
>> 
>> Maybe there's some non-obvious mistake. Implementing it directly in
>> qprocess_win.cpp may make it work.
> 
> Ok, thanks for taking a look, I will see if I find the error or perhaps 
> somebody
> else has input, too.
> 
> At the moment I am just a bit confused what it might be, I did some 
> errors
> before I arrived with that, but then at least I got some API errors 
> back,
> now just the output from my called process won't arrive.

Just for future references, the issue was a duplicated file descriptor
passed to the function if channels are merged.

This works:

     // ensure the child process doesn't inherit our file handles
#ifdef Q_OS_WIN
     STARTUPINFOEX info;
     ZeroMemory(&info, sizeof(info));
     
m_process.setCreateProcessArgumentsModifier([&info](QProcess::CreateProcessArguments 
*args) {
         // only keep the handles that we have in args->startupInfo
         SIZE_T size = 0;
         LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList = NULL;
         UR_ENSURE(InitializeProcThreadAttributeList(NULL, 1, 0, &size) 
|| (GetLastError() == ERROR_INSUFFICIENT_BUFFER));
         UR_ENSURE((lpAttributeList = 
reinterpret_cast<LPPROC_THREAD_ATTRIBUTE_LIST>(HeapAlloc(GetProcessHeap(), 
0, size))));
         UR_ENSURE(InitializeProcThreadAttributeList(lpAttributeList, 1, 
0, &size));

         // ensure all handles are inheritable
         for (auto handle : {args->startupInfo->hStdInput, 
args->startupInfo->hStdOutput, args->startupInfo->hStdError}) {
             UR_ENSURE(SetHandleInformation(handle, HANDLE_FLAG_INHERIT, 
HANDLE_FLAG_INHERIT));
         }

         const auto numberOfUniqueHandles = 
(args->startupInfo->hStdOutput == args->startupInfo->hStdError) ? 2 : 3;
         UR_ENSURE(UpdateProcThreadAttribute(
             lpAttributeList, 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, 
&args->startupInfo->hStdInput, numberOfUniqueHandles * sizeof(HANDLE), 
NULL, NULL));

         info.StartupInfo = *(args->startupInfo);
         info.StartupInfo.cb = sizeof(info);
         info.lpAttributeList = lpAttributeList;
         args->flags |= EXTENDED_STARTUPINFO_PRESENT;
         args->startupInfo = &info.StartupInfo;
     });
#else
     QProcess::UnixProcessParameters params;
     params.flags = QProcess::UnixProcessFlag::CloseFileDescriptors;
     params.lowestFileDescriptorToClose = 0;
     m_process.setUnixProcessParameters(params);
#endif

     // dispatch to internal process
     m_process.start(program, arguments, mode);

#ifdef Q_OS_WIN
     // cleanup only if the setCreateProcessArgumentsModifier functor was 
executed
     if (info.lpAttributeList) {
         DeleteProcThreadAttributeList(info.lpAttributeList);
         HeapFree(GetProcessHeap(), 0, info.lpAttributeList);
     }
#endif

Greetings
Christoph

> 
> Greetings
> Christoph
> _______________________________________________
> Interest mailing list
> Interest at qt-project.org
> https://lists.qt-project.org/listinfo/interest