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

[Refactor] Unify the invocation of external processes and properly grab STDOUT and STDERR #95

Closed
GerHobbelt opened this issue Oct 6, 2019 · 2 comments
Labels
🐛bug Something isn't working 🕵investigate Needs further analysis to find the root cause. 🕵TLC Needs some special attention
Milestone

Comments

@GerHobbelt
Copy link
Collaborator

Been looking at the code for a while now and there's plenty places where external applications are executed, each with its own special flavor of code (probably as experience grows and history has its way 😄).

Meanwhile I'm pondering 🤔 that this is nice to have for running JavaScript scripts and bash/powershell scripts on PDFs and metadata so that I could integrate the nice parts of my old reverse engineering and work around Qiqqa project and PDF-in-the-Wild nastinesses.

Anyway, today the bubble popped and this issue got filed. Case in point:

QiqqaOCR generates a fault and the logging cannot dump its report properly because STDERR just so happens to have gone the way of the Dodo. Sigh. And if it just had said in there: "Exception bla bla, no words to write ladidah" it would have saved me some time.

Debugging QiqqaOCR with the same GROUP parameters gives:

The thread 0x264 has exited with code 0 (0x0).
Exception thrown: 'System.Exception' in QiqqaOCR.exe
We have no wordlist to write!

🎉 tada! 🎉

@GerHobbelt
Copy link
Collaborator Author

GerHobbelt commented Nov 3, 2019

Standard idiom now looks something like this:

using (Process process = ProcessSpawning.SpawnChildProcess(ConfigurationManager.Instance.Program7ZIP, process_parameters, ProcessPriorityClass.Normal))
{
  using (ProcessOutputReader process_output_reader = new ProcessOutputReader(process))
  {
    process.WaitForExit();

    Logging.Info("XULRunner installer:\n{0}", process_output_reader.GetOutputsDumpString());
  }
}

GerHobbelt added a commit to GerHobbelt/qiqqa-open-source that referenced this issue Nov 3, 2019
…riginal Qiqqa. Upon closer inspection the remaining `Process.Start()` calls are are intended to open an (associated) application for a given file or directory, which is proper.

Added a few `using(...)` statements around Process usage, etc. to prevent memory leaks on these IDisposables.
GerHobbelt added a commit to GerHobbelt/qiqqa-open-source that referenced this issue Nov 3, 2019
@GerHobbelt
Copy link
Collaborator Author

Done for now AFAIAC

TODO list automation moved this from In progress to Done Nov 3, 2019
@GerHobbelt GerHobbelt moved this from To do to Done in v82pre5 Nov 3, 2019
@GerHobbelt GerHobbelt moved this from To do to Done in Scripting Qiqqa Nov 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug Something isn't working 🕵investigate Needs further analysis to find the root cause. 🕵TLC Needs some special attention
Projects
TODO list
  
Done
v82pre5
  
Done
Development

No branches or pull requests

1 participant