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

scala.sys.process.ProcessImpl.PipedProcess.destroy() does not clean up pipe threads #7350

Closed
scabug opened this issue Apr 9, 2013 · 5 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented Apr 9, 2013

If you create a process with either input or output piped into an InputStream or OutputStream, and destroy the process before it terminates naturally, two threads are created but not destroyed (two per pipe, so if both input and output are piped, this comes to four threads).

The issue does not occur if the process is allowed to terminate of its own accord.

The cause of the issue appears to be the variables currentSink and currentSource. If the process is allowed to terminate naturally, these are set to None, signalling to the pipe processes that they should exit (lines 135-138 in processImpl.scala). However, if the process is terminated by the destroy() method, no such cleanup is done (lines 143-144).

I've attached a script that triggers the issue.

I'm working on a patch for this, but I'm a first time contributor to the scala codebase, so it might take a while to familiarise myself with the process.

@scabug
Copy link
Author

scabug commented Apr 9, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7350?orig=1
Reporter: James Pickering (james_pic)
Affected Versions: 2.10.1
Attachments:

@scabug
Copy link
Author

scabug commented Apr 12, 2013

@adriaanm said:
scala/scala#2379

@scabug
Copy link
Author

scabug commented May 20, 2013

@JamesIry said:
2.10.2 is about to be cut. Kicking down the road and un-assigning to foster work stealing.

@scabug
Copy link
Author

scabug commented Sep 1, 2014

@gkossakowski said:
scala/scala#3920

@scabug scabug closed this as completed Sep 1, 2014
@scabug
Copy link
Author

scabug commented Sep 1, 2014

@rubyu said:
I think #8768 would also be fixed by scala/scala#3920

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

2 participants