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

'run' hangs on large output #52

Closed
ileitch opened this issue Feb 11, 2018 · 18 comments
Closed

'run' hangs on large output #52

ileitch opened this issue Feb 11, 2018 · 18 comments

Comments

@ileitch
Copy link

ileitch commented Feb 11, 2018

Using version 4.0.0 this commad hangs: run(bash: "for i in {1..100000}; do echo -n =; done")

screen shot 2018-02-11 at 5 19 19 pm

@kareman
Copy link
Owner

kareman commented Feb 12, 2018

This is a very interesting bug.

  • "for i in {1..100000}; do echo -n =; done" works fine in the terminal.
  • the program hangs when run in SwiftShell if we wait for the command to finish before reading from its output. "run" does this. "runAndPrint" does not and works fine. So does "runAsync". Unless you run it like this:
let l = runAsync(bash: "for i in {1..65537}; do echo -n =; done")
try l.finish()
print(l.stdout)
  • If we change the number to 65536 it works fine. If we increase it to 65537 it hangs. This means the bug only occurs if the output contains more than 2^16 characters, or in other words has a count that cannot be stored in 2 bytes.
  • I have no idea why that is.

I think this bug is the same as #15. We can't use the same fix used then because "run" has been redesigned since then and has to finish before reading its output. I want it to be possible to use "run" without reading any output at all.

Here is a workaround:

_ = runAsync(bash: "for i in {1..65537}; do echo -n =; done").stdout.read()

This will make sure the output of the command has been closed (and therefore the command has finished running) before continuing.

@kareman
Copy link
Owner

kareman commented Feb 13, 2018

The problem is described here: https://stackoverflow.com/a/39281558/96587
And here: https://github.com/lroathe/PipeTest

It happens when the output pipe from the command is full. So the problem is with Foundation's Process and FileHandle classes. Not really much we can do about it in SwiftShell except for informing users about it. I'll add it to the README.

@kareman kareman changed the title run hangs on large output 'run' hangs on large output Feb 13, 2018
@ileitch
Copy link
Author

ileitch commented Feb 13, 2018

I want it to be possible to use "run" without reading any output at all.

Would it be possible to read the output, but not necessarily return it unless requested? Perhaps it'd incur a small performance hit, but this issue makes run pretty dangerous to use. It may be extremely difficult for users to predict future output sizes when choosing to use this method, or large output might only be generated in exceptional circumstances. IMO your library should "just work" vs. causing developers lost sleep when their program startings hanging randomly in the middle of the night 😉

@kareman
Copy link
Owner

kareman commented Feb 13, 2018

You’re right, this is pretty dangerous. There doesn’t seem to be any way of increasing the buffer size either.

To reduce overhead we could store the output as just Data, and convert to String only upon request.

One problem though: we might have to read from stdout and stderror simultaneously. Because if we try to read all output from stdout in one go and the command fills up stderror (or the other way around), the command will still hang.

I won’t be able to implement this for a couple of weeks, any pull requests are welcome if you want to do it.

@kareman
Copy link
Owner

kareman commented Feb 23, 2018

@ileitch Actually i don’t think stderror should ever contain more than 65KB (I mean how much error information can a command produce?). Do you think it would suffice to just read all Data from stdout before waiting for command to finish?

@ileitch
Copy link
Author

ileitch commented Feb 24, 2018

I don't think it's safe to make assumptions about what amount of stderr output might be produced. I'm of the opinion that this method should not hang under any circumstance.

@kareman
Copy link
Owner

kareman commented Feb 25, 2018

@ileitch I have tried to fix this problem in #54, can you try it out and see if it works? This is probably not the most efficient way of doing this, but I want to be absolutely sure we don't run into any multi threading issues.

@Ponyboy47
Copy link
Contributor

I think I too have been running into this issue when using Swift Shell to run some Handbrake transcoding commands (there's a whole lot of output even without it running verbosely). I am ending up with several zombied Handbrake processes and my program is left hanging. I have verified that it is not issuing a log command immediately after my SwiftShell.run() and so I suspect I too am hitting this problem.

Because of how I am using the result of the SwiftShell.run(), it would not be easy for me to use the runAsync workaround.

I am testing the fix in #54 right now to see if it resolves my issue.

@Ponyboy47
Copy link
Contributor

I was previously using my own homemade class for running shell commands which worked, but I wanted a bit more power/functionality and didn't want to build it myself. That's when I found this framework and just recently started using it instead. It has the same clean usage as my homemade solution, but it far more powerful/organized and meant I've got one less thing to manage myself.

As soon as I switched to this though, I started noticing my handbrake commands were frequently hanging or becoming zombies and my program would hang. Due to my busy schedule I hadn't had time to look into it until today, which is when I saw this issue and thought it may be related to my problem.

@kareman
Copy link
Owner

kareman commented Feb 28, 2018

@ileitch @Ponyboy47 Have you had a chance to test out pull request 54? Did it work?

@Ponyboy47
Copy link
Contributor

Sorry I forgot to respond after testing this.

Yes the fix in #54 did allow my program to continue execution as expected :)
Thank you for identifying and creating this fix!

@ileitch
Copy link
Author

ileitch commented Mar 2, 2018

Sorry I haven't tried it. I stopped using SwiftShell when I discovered this issue. The PR looks good though 👍🏼

@kareman
Copy link
Owner

kareman commented Mar 4, 2018

Excellent, I have merged the PR and will release a new version as soon as the CI tests pass.

@patchthecode
Copy link

The finish command says this:

Screen Shot 2020-09-16 at 3 58 15 PM

However, when i do a asyncCommand?.stdout.read() as instructed, it takes a very long time to read and complete. Essentially, it still hangs the app. After like about 2 mins, it finally returns.

@kareman
Copy link
Owner

kareman commented Sep 16, 2020

Could you post the code, including the shell command?

@patchthecode
Copy link

let asyncCommand = SimulatorCommand.executeBuild(xctestrunFilePath, onSimulatorWithId: self.simulator.id) { value in
    print("The command has truly ended")
}
asyncCommand.stop()
_ = try? asyncCommand.finish()
_ = asyncCommand.exitcode()
asyncCommand.stdout.read()
asyncCommand.stderror.read()

This is the code i ran.
The file that is running is in the command is the xcode simulator.
Basically, i wanted the stop command to stop immediately so that the print statement will be printed.
But it hangs a but on the both read commands. Before exiting.

@patchthecode
Copy link

patchthecode commented Sep 16, 2020

The first line runs your runAsync commmand under the hood.

[Edit] - let me check again. I use the nohup ... & command under the hood. Let me check and see if this is whats causing this.

@patchthecode
Copy link

OK it looks like this had something to do with it.
Ignore my previous text.

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

No branches or pull requests

4 participants