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

Using sys.process results in file descriptor leaks #5439

Closed
scabug opened this issue Feb 6, 2012 · 12 comments
Closed

Using sys.process results in file descriptor leaks #5439

scabug opened this issue Feb 6, 2012 · 12 comments
Assignees
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Feb 6, 2012

Using scala.sys.process results in file descriptor leaks in the form of pipes used internally by the library. For most usages, there's no workaround possible.

The issue is that the streams associated with each SimpleProcesss are not closed, and the user can't get at them. This issue has been fixed on SBT, but that fix results in another bug. I'm investigating the issue.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Feb 6, 2012

Imported From: https://issues.scala-lang.org/browse/SI-5439?orig=1
Reporter: @dcsobral
Affected Versions: 2.9.0
Other Milestones: 2.10.0

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Feb 6, 2012

@dcsobral said:
The SBT patch -- which closes the streams at SimpleProcess results in this exception being thrown:

  [partest] java.io.IOException: Stream closed
  [partest] 	at java.io.BufferedInputStream.getBufIfOpen(BufferedInputStream.java:145)
  [partest] 	at java.io.BufferedInputStream.read1(BufferedInputStream.java:255)
  [partest] 	at java.io.BufferedInputStream.read(BufferedInputStream.java:317)
  [partest] 	at java.io.FilterInputStream.read(FilterInputStream.java:90)
  [partest] 	at scala.sys.process.BasicIO$.loop$1(BasicIO.scala:115)
  [partest] 	at scala.sys.process.BasicIO$.transferFullyImpl(BasicIO.scala:122)
  [partest] 	at scala.sys.process.BasicIO$.transferFully(BasicIO.scala:104)
  [partest] 	at scala.sys.process.ProcessImpl$PipeThread.runloop(ProcessImpl.scala:159)
  [partest] 	at scala.sys.process.ProcessImpl$PipeSource.run(ProcessImpl.scala:180)

PipeSource is one of the two threads that copy the output of one process to another when doing something like p1 #| p2. When p1 loses its threads, that causes the exception above. p1 doesn't know about PipeSource, as it is a SimpleProcess. The object that knows about p1, p2 and PipeSource, the PipedProcesses, doesn't have a way to stop PipeSource.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Feb 7, 2012

@adriaanm said:
Daniel, looks like you're on top of this, so might as well give you credit for it :-)

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Feb 20, 2012

@acruise said:
I believe this should be marked resolved? scala/scala@838c97b

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Feb 21, 2012

@dcsobral said:
Yes. Oops, I'm the owner of this, am I not? Ok... :-)

Fixed by 838c97bcb55908aff3638caae0aa87d237100b4b.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Feb 21, 2012

@scabug scabug closed this Feb 21, 2012
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 28, 2012

@lrytz said:
thanks for the fix - we have a play application that polls the scala github repo for new commits (it calls "git pull" and "git rev-list" on a local checkout). this bug makes it crash every two weeks or so.. if only it was backported to 2.9.x :(

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 28, 2012

@dcsobral said:
@lukas I'm pretty sure it was backported to 2.9.2!

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 28, 2012

@dcsobral said:
Indeed, it was backported to 2.9.x on commit 28be69e2637b935de9320a6a60bde25e62acddab, and is part of Scala 2.9.2.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 28, 2012

@lrytz said:
oh, thanks - then my git fu failed me.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 28, 2012

@acruise said:
So, Lukas, is the process that's crashing every two weeks running 2.9.2? :)

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 28, 2012

@lrytz said:
i did not test with 2.9.2. i quickly googled for "play" and "2.9.2" and found a few posts that changing the scala version to 2.9.2 is at least not trivial and might not work at all - but i don't have time right now to try. i did a hack around instead (when i didn't know yet that 2.9.2 would work): [https://github.com/lrytz/scalabuilds/commit/1cc8051338c83ef47710f78ccf63ed478441899c]

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.