Page MenuHomePhabricator

Do not create new terminals when launching process on Windows with --no-stdio
ClosedPublic

Authored by xbolva00 on Sep 12 2018, 12:57 AM.

Diff Detail

Repository
rLLDB LLDB

Event Timeline

xbolva00 created this revision.Sep 12 2018, 12:57 AM
xbolva00 edited the summary of this revision. (Show Details)Sep 12 2018, 1:01 AM
xbolva00 added a reviewer: stella.stamenova.
labath added a subscriber: labath.Sep 12 2018, 1:30 AM

I think the issue here is that on windows we don't have the ability(*) to pipe stdio through lldb, so if you create a process without a console, you will not be able to use see it's output or pass it some input (probably fine for gui apps, but bad for console ones). This means your approach will also make things inconsistent with other platforms, only in a different way. (I don't really have an opinion which one is better/worse.)

(*) I think there is a way to emulate the unix pty behavior on windows, but it is not exactly straight-forward. It involves creating a hidden console and then polling that for changes/keypresses (I don't know the details, but that's how someone once explained it to me).

xbolva00 abandoned this revision.Sep 12 2018, 1:40 AM

I think the issue here is that on windows we don't have the ability(*) to pipe stdio through lldb, so if you create a process without a console, you will not be able to use see it's output or pass it some input (probably fine for gui apps, but bad for console ones). This means your approach will also make things inconsistent with other platforms, only in a different way. (I don't really have an opinion which one is better/worse.)

(*) I think there is a way to emulate the unix pty behavior on windows, but it is not exactly straight-forward. It involves creating a hidden console and then polling that for changes/keypresses (I don't know the details, but that's how someone once explained it to me).

Ok, thanks for info!

We should obey the --tty option here and set CREATE_NEW_CONSOLE if it is set no?

xbolva00 added a comment.EditedSep 12 2018, 7:55 AM

We should obey the --tty option here and set CREATE_NEW_CONSOLE if it is set no?

I also thought about it and I am in favour of this solution. Suggestions welcome, especially from @zturner, @labath, @JDevlieghere , @teemperor, ..

The other option is to only remove this flag if "--no-stdio" is set.

The best solution IMHO is:

  • find a way to get stdio from a process and feed to to LLDB so it shows up when neither --tty nor --no-stdio is set
  • when --tty is specified, set the CREATE_NEW_CONSOLE bit
  • when --no-stdio is specified, don't get stdio from child process and don't set CREATE_NEW_CONSOLE

So basically the hardest problem is to "find a way to get stdio from a process and feed to to LLDB so it shows up when neither --tty nor --no-stdio is set" ..

So basically the hardest problem is to "find a way to get stdio from a process and feed to to LLDB so it shows up when neither --tty nor --no-stdio is set" ..

yes. The easier solution is to just create a console for now if this is too hard. So the fix here would be to just not set the CREATE_NEW_CONSOLE if --no-stdio is set.

xbolva00 updated this revision to Diff 165103.Sep 12 2018, 9:35 AM
xbolva00 retitled this revision from Do not create new terminals when launching process on Windows by default to Do not create new terminals when launching process on Windows with --no-stdio.
xbolva00 edited the summary of this revision. (Show Details)

This makes sense to me, but the windows folks should say what is best for their platform.

zturner accepted this revision.Sep 12 2018, 9:54 AM

I'm fine with this.

This revision is now accepted and ready to land.Sep 12 2018, 9:54 AM
This revision was automatically updated to reflect the committed changes.