Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Console error then open some extension file. #495

Closed
T-Troll opened this issue Dec 6, 2021 · 22 comments
Closed

Console error then open some extension file. #495

T-Troll opened this issue Dec 6, 2021 · 22 comments
Labels

Comments

@T-Troll
Copy link

T-Troll commented Dec 6, 2021

Far Manager version

3.0.5888.0 x64

OS version

10.0.19044.1387

Other software

(maybe) WireShark.

Steps to reproduce

  1. Select any file with .pcap (wireshark log) extension.
  2. Press Enter.

Expected behavior

File opened in Wireshark.

Actual behavior

File opened in Wireshark, but also console error in Far, followed by visual mess (panel position shifts up by console error text size (1-2 rows)).

@T-Troll T-Troll added the bug label Dec 6, 2021
@alabuzhev
Copy link
Contributor

Sit beside me and hear my sad story.

There are apps that talk to the the outside world in text (for example, git). When you run them, either the console of the parent process is used (if any), or a new one is created ("the scary black window").

There are programs that talk to the outside world in various GUI controls (for example, photoshop or wireshark). They do not need the console by design, it only scares the users, so such apps are linked in a special way and when you run them, this scary black window is not created.

And there are libraries that usually don't talk to the outside world at all.

From time to time developers want to show various debug messages for various reasons.
For text apps it's easy - you have the console and can print anything directly into it.
For GUI apps and libraries it's a little more complicated - there is no console, and adding proper logging apparently takes too much time and effort.

Once upon a time, probably after weeks of meditation, one software guru discovered that he can attach to the console of the parent process and drop his stuff right into it! Just imagine how convenient it is - laypeople, who run his piece of work from Windows Explorer, won't notice anything suspicious, because it does not have a console, but he can neatly run it from cmd.exe and see everything. Awesome.

Another day another specimen discovered that in the library code you can just write to stdout without bothering with anything at all. If there's any console attached to the process, the text will be visible. And if no, then not. Amazing.

Gurus share their wisdom on stackoverflow.com and the sacred knowledge goes to the masses.

And the fact that the console might not even belong to them in the first place, and can be actively used by something else - meh, who cares.

In other words:

  • Wireshark.exe is linked as a GUI application.
  • Therefore it is not supposed to output anything.
  • Its developers have utilised the magic trick above - they manually attach to the parent's console and litter it up with their debug messages, causing the visual mess you see.

Obviously, we can't do anything about it.
If this behavior annoys you, please talk to wireshark developers directly and ask them to stop doing that, or at least not do that by default.

@alabuzhev alabuzhev added external and removed bug labels Dec 7, 2021
@T-Troll
Copy link
Author

T-Troll commented Dec 7, 2021

Uh-oh.

Спасибо, Алекc, it's quite a full explanation.
You are right, some developers (especially cross-patform ones), have no idea Windows have special mechanics for this - debug output, visible into WinDbg and other tools.

But i can't agree we have no method to avoid this behavior:

  1. We can start an external application not from current shell, but from new shell with SW_HIDE attribute (i'll do this in my GUI apps). Minus - we can't get application output if this is a console application, but it will work well for most extension-driving launches.
  2. Refresh our console window, then focus return into it (hook focus). It will hide the mess if any.

PS: I also remember other bit annoying (and connected) issue - if i have a scroll bar into Far console window (f.e. for history), close panels by CTRL+O and scroll it back, then press CTRL-O again to show it back - i still stay at current scrolling position, with panels at the end of console. Is it possible to scroll to the end in this case, make panels visible without manual scrolling?

@alabuzhev
Copy link
Contributor

But i can't agree we have no method to avoid this behavior

There are workarounds of course, it's just that my comment got too long already. By "we can't do anything" I mean "anything remotely reasonable, that can be added to the code and won't break something else".

not from current shell, but from new shell with SW_HIDE attribute

Yes, anything that would break the parent-child process link will do.

it will work well for most extension-driving launches

What about bat files, python scripts etc.?

Refresh our console window, then focus return into it (hook focus). It will hide the mess if any.

It might help if the offending app litters once. What if it does so every second?


Actually, everything you need is already there, you just need to set it up manually:

  • F9 - commands - file associations - Ins
  • mask: *.cap
  • execute command: start "" !.!

This way far will spawn cmd that will spawn wireshark and quit.


Is it possible to scroll to the end in this case, make panels visible without manual scrolling?

Just press Esc.

@T-Troll
Copy link
Author

T-Troll commented Dec 7, 2021

What about bat files, python scripts etc.?

You are right, it will need to separate executable and extension. Ok, forget about it.

It might help if the offending app litters once. What if it does so every second?

Well... Better than nothing. It can also help in other situations (like moving windows to the different DPI screen, it made a mess sometimes right now).

execute command: start "" !.!

I think also /MIN. Unfortunately, start does not support hidden window, but this is a way, i'll follow it.

Just press Esc.

Thank you for the hint. But did it reasonable to keep scrolling outside panel then user issue "show me panels" command?

PS: Ok, the situation is clear for me, and for you as well. Please, think a bit about my points, i'm close this issue.

@T-Troll T-Troll closed this as completed Dec 7, 2021
@alabuzhev
Copy link
Contributor

moving windows to the different DPI screen, it made a mess sometimes right now

Please raise a new issue for this.

But did it reasonable to keep scrolling outside panel then user issue "show me panels" command?

I'll think about it.

@elfmz
Copy link

elfmz commented Dec 12, 2021

You may create small intermediate GUI application to start needed GUI applications. So that sacred hack will not find your console (unless they're really persistent and will try to go deeper to upper parents). That intermediate app then may either wait for its target completion and exit with same exit code, or if FAR doesn't care about all that - do WaitForInputIdle and exit.

@T-Troll
Copy link
Author

T-Troll commented Dec 12, 2021

@elfmz The question is to decide what extension need this kind of start and what didn't, and it will interfere with output.

@elfmz
Copy link

elfmz commented Dec 12, 2021

It must be used only for applications that have win32 type specified in PE header.

@T-Troll
Copy link
Author

T-Troll commented Dec 12, 2021

It's a mess here as well. Application type is wrong sometimes.

@alabuzhev
Copy link
Contributor

create small intermediate GUI application to start needed GUI applications

We do not know beforehand whether a given command spawns a GUI or a CUI application. We just run it and get a process handle, which we then inspect, find out its subsystem and act accordingly.
So injecting an intermediate app is not feasible with this approach, at least until time travel is generally available.

@elfmz
Copy link

elfmz commented Dec 12, 2021

Then, if CreateProcess is used - may be this may help - https://scorpiosoftware.net/2021/01/10/parent-process-vs-creator-process/ - it allows to specify arbitrary parent for launched process. Unfortunately documentation (https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-updateprocthreadattribute) doesn't tell anything how it will affect behavior of console processes - if they will use console of that surrogate parent or still will run within console of actual parent, so its up to checking on practice. But if it will not affect console inheritance - then you may run simple win32 app and just assign it as parent for all processes you're starting.

@alabuzhev
Copy link
Contributor

Thank you, it's a quite interesting trick.
I've quickly experimented and it looks like it does not affect the console inheritance (which is not surprising, giving that ProcessConsoleHostProcess info class exists):

image

A GUI app that invokes AttachConsole(ATTACH_PARENT_PROCESS) gets fooled successfully and invades the fake parent.

Unfortunately, all console processes spawned this way immediately fail with STATUS_DLL_INIT_FAILED, supposedly more magic is required for such initialisation.

And, unfortunately, this won't help in this particular case at all, since CreateProcess doesn't work with file associations and they are handled by ShellExecuteEx.

@alabuzhev
Copy link
Contributor

And, obviously, the parent-child relation is a good thing and a useful concept in general, e.g. finding a specific instance is much easier in a hierarchical structure than in a flat list, especially if you have hundreds of processes. I don't think we should break it for everything and everyone only because a few specific apps do mental things.

Again, this is not a Far-only issue, all console processes are affected. The user might start wireshark from a cmd session and then immediately start doing something else in the same session and that debug spam could break their workflow.
The whole "attach to parent" concept is inherently flawed. You can't do that unless you control the parent, know exactly what it does at the moment and can guarantee that you won't break it with your actions. Using an arbitrary parent's console for debug logs is a bug that should be reported and addressed, not workarounded.

@elfmz
Copy link

elfmz commented Dec 12, 2021

I suspect it will be hard to convince that its a bug as MS added this as explicit documented flag for AttachConsole -ATTACH_PARENT_PROCESS. And i dont remember this flag from times i was Windows dev, likely it was added recently - that guru must be working at MS.

@alabuzhev
Copy link
Contributor

The existence of an API doesn't yet imply that it can be carelessly used for arbitrary actions.
As I said, it could be perfectly fine if you control the parent and know what it does, e.g. if it's your own "debug launcher", created for exactly this purpose - provide an optional console for your GUI apps.

DeleteFile and TerminateProcess also exist and fully documented, does that mean that an app can delete random files or kill random processes if there's non enough disk space or memory for it?

@bitraid
Copy link
Contributor

bitraid commented Dec 12, 2021

One possible solution that could be used with CreateProces to enable GUI programs to write to console without breaking anything, even those that don’t capture the parent console (many cross-platform GUI programs expect to be able to write to the console if started from there – for example if you run Microsoft’s own extrac32.exe from MSYS2 console there will be text output, whereas in CMD there won’t be any unless you use something like extrac32 | more):

  • Create two inheritable anonymous pipes
  • Create the target process with the write end of the pipes for its stout, stderr
  • Close the write handles of the pipes and start two threads that read the pipes and write to stdout/stderr
  • Wait for threads, close handles etc.
    This is what I used for a POC launcher on a similar discussion. It’s a quick and dirty implementation written in 32-bit assembly (compiled with fasm) and will probably make your AV have panic attack. But it works (must have the same name as the GUI program and .com extension) and can be used with console programs without issue.

@alabuzhev
Copy link
Contributor

it will be hard to convince that its a bug

There's another argument, if "common sense" is not enough:

AttachConsole doesn't attach a console to your app, it attaches your whole app to the console.
Including control handlers.
In other words: the user closes the console, your app gets terminated. Oops.

@elfmz
Copy link

elfmz commented Dec 12, 2021

Alternatively, if such process attached to console and writing to it - it can be considered as console app with GUI. You may detect such situation by checking if spawned process appeared in GetConsoleProcessList and.. do not return until it finish (or disappear from GetConsoleProcessList). Not sure whats better though - see random console garbage or this. Also, race possible if that process will attach after you check for that (but may guess that this will not happen since ShellExecute does WaitForInpuIdle - likely spawned process will attach console before starting to pump events.
And for Shift+Enter execs (and whatever else similar) - use intermediate application as described in my 1st message here.

@alabuzhev
Copy link
Contributor

Not sure whats better though - see random console garbage or this

Yes, exactly - random GUI apps blocking the execution would cause even more raised eyebrows and bug reports.

@elfmz
Copy link

elfmz commented Dec 12, 2021

Anyway, adding intermediate app for Shift+Enter can be useful at least as workaround for such cases. As i understand currently there is no workaround except yelling to authors of nasty app?

@MKadaner
Copy link
Contributor

I wonder how many such nasty apps are there? FAR can nicely handle majority of cases. There are few corner cases. Maybe instead of having FAR authors to handle those cases and run the risk of breaking some other mainstream cases, the avid users of those applications can apply targeted workaround? Such workaround may be promoted to a macro or a plugin if it is good enough. Then, all users of such nasty apps would enjoy running them seamlessly from FAR, while other users will not pay for overhead on launching every command?

@alabuzhev
Copy link
Contributor

there is no workaround except yelling to authors of nasty app?

I suppose one can write a macro for Shift+Enter that would take the command line text and pass it to any desirable external launcher, or at least to cmd with the "start" prefix.

Of course, it would've been nice to have an out-of-the-box solution at least for Shift+Enter, which is a slightly less convoluted case since we don't need to care about GUI/CUI at all.
However, as @MKadaner said, this could break more mainstream cases in one way or another, and the previous experience tells me that it absolutely will - even legit & reasonable changes break things (obligatory XKCD).

It is easy to come up with various workarounds for specific cases, but much harder to provide a robust generic solution. I see this issue from time to time since 2010 or so, think about it every time but can't find a silver bullet.

And again, this is a bug and it should be fixed. Here Far is not even mentioned, yet people are suffering from the same issue in pure cmd because some electron dev was way too smart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants