[PATCH 2/2] kernelbase: Add support for CREATE_NO_WINDOW flag.

[Jacek Caban]

On 4/5/22 17:00, Eric Pouech wrote:
> Le 05/04/2022 à 14:34, Jacek Caban a écrit :
>>
>> Based on patches by Eric Pouech and Torge Matthies.
>>
>> Signed-off-by: Jacek Caban <jacek at codeweavers.com>
>> ---
>> Both Eric and Torge sent patches for this and both used ConsoleFlags 
>> for passing the information. While reviewing both series, I was 
>> wondering if using ConsoleHandle instead would be cleaner and I think 
>> it is. I ended up having a complete third version...
>>
>> Eric, Torge, I hope you will review this.
>>
>>  dlls/kernel32/tests/console.c |  3 ---
>>  dlls/kernelbase/console.c     | 21 +++++++++++++++------
>>  dlls/kernelbase/process.c     | 11 +++++++----
>>  include/wine/condrv.h         |  5 +++--
>>  programs/conhost/conhost.c    | 11 ++++++++---
>>  programs/conhost/conhost.h    |  1 +
>>  6 files changed, 34 insertions(+), 18 deletions(-)
>>
> two comments:
>
> - the value for ConsoleHandle in child process for a GUI exec is not 
> consistent depending on passed flags:
>
>   + it's going to be 0 value when CREATE_NO_WINDOW is passed
>
>   + it's value is untouched when 0 is passed (at least none of 
> CREATE_NO_WINDOW, DETACH_PROCESS, CREATE_NEW_CONSOLE)
>
> under windows, it's always zeroed out (as the remaining todo_wine in 
> test_CreateProcessCUI shows)
>
> the attached (modified) patch implements this behavior; but it's quite 
> a change...


Yes, it's quite a change, but since we already opened that can of worms, 
I think we should handle that case properly as well. Handling GUI 
processes deserves a separated patch, I was hoping that you could follow 
up with that fix.


> - just for the record: I wonder if in dlls/ntdll/unix/process.c we 
> shouldn't also detach from the unix controling terminal in 
> CONSOLE_HANDLE_ALLOC_NO_WINDOW case (but it would be impacting with 
> next patch as ctrl-c would no longer be working). so I'd say no
>

Good point, I missed that part.


> this 'next' - aka third - patch,  just creates a window less console 
> in some cases for an initial process and should fix (or at least 
> improve) a couple of open bugs
>
> (to make it short, in scenario like "wine foo >& log < /dev/null", it 
> prevents foo and the subprocesses when in cui subsystem to create 
> visible consoles)
>
> (just to share it here)
>

To recap our private conversation, there are nice things about that 
solution: it makes such invocation more similar to a more common 
situation on Windows and I like that aspect. My main concern is that it 
will make invoking Wine in situations when there is no Unix console 
available much more expensive, because we'd need to create an extra 
conhost process that would be useless in majority of cases. I know that 
we have some users using Wine as part of their Unix scripts, likely ran 
in the background, for whom it's likely to provide worse performance due 
to need to create twice as many Wine processes. It may not be a deal 
breaker, but I think it's a concern that we should consider.


One possible solution to that would be, for example, to have something 
like CONSOLE_HANDLE_SHELL_NO_WINDOW, which would be inherited by 
CreateProcess(0) and prevent from creating an actual new console. This 
would achieve most of this patch's goals with lower runtime price.


Other possible solution is more specific to Wine tests, which are 
causing most of those problems. We could just build tests as GUI 
applications and avoid the problem in the first place. It's not a 
general solution, through. We could maybe have wineconsole --no-window 
option that, instead of using AllocConsole, would use CREATE_NO_WINDOW. 
This would essentially give user an opt-in option to achieve the same 
outcome as your patch.


Anyway, none of above is perfect. I just thrown more ideas to the pool, 
hoping for your and/or other's opinions.


I think that the root of the problem is that there is no single right 
thing when Wine is invoked by Unix process with no Unix console 
available (when Unix console is available, the choice to use it is 
mostly uncontroversial). In some cases, creating a console window is 
desired, while for others it's not. Currently user is able to force 
console window creation by running explicitly wineconsole, but the 
opposite is not possible under assumption that "default" is good for 
other cases. This is an equivalent of creating the process with 
DETACHED_PROCESS on Windows.


Thanks,

Jacek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20220405/797ec842/attachment.htm>