[CORE][MINOR]Closes stream and releases any system resources associated with this stream #18522
Conversation
| } { | ||
| closeFile() | ||
| try { | ||
| inputStream.close() |
There was a problem hiding this comment.
AFAIK this inputStream is passed from outside, like process.getInputStream in ExecutorRunner, so I don't think we should close the inputStream here.
There was a problem hiding this comment.
Yes,you are right.
But this function is only used in ExecutorRunner, also if an exception occurs within this function,this will ensure the inputStream is closed
There was a problem hiding this comment.
The another reason is that this function runs in another thread
There was a problem hiding this comment.
It shouldn't be closed here, because the lifecycle of this stream is outside of this class. In practice, the streams used with this class are closed on error anyway.
There was a problem hiding this comment.
I referred to DriverRunner:
DriverRunner-->runDriver-->redirectStream-->copyStream, the inputStream is closed in copyStream.
|
Test build #79120 has finished for PR 18522 at commit
|
| test("resolveURIs with multiple paths") { | ||
| def assertResolves(before: String, after: String): Unit = { | ||
| assume(before.split(",").length > 1) | ||
| assume(before.split(",").length >= 1) |
There was a problem hiding this comment.
When running UtilsSuite, the following warning occurred:
Test Canceled: 1 was not greater than 1
org.scalatest.exceptions.TestCanceledException: 1 was not greater than 1
at org.scalatest.Assertions$class.newTestCanceledException(Assertions.scala:511)
at org.scalatest.FunSuite.newTestCanceledException(FunSuite.scala:1555)
at org.scalatest.Assertions$AssertionsHelper.macroAssume(Assertions.scala:481)
at org.apache.spark.util.UtilsSuite$$anonfun$19.assertResolves$2(UtilsSuite.scala:491)
at org.apache.spark.util.UtilsSuite$$anonfun$19.apply$mcV$sp(UtilsSuite.scala:512)
at org.apache.spark.util.UtilsSuite$$anonfun$19.apply(UtilsSuite.scala:489)
at org.apache.spark.util.UtilsSuite$$anonfun$19.apply(UtilsSuite.scala:489)
10110346
changed the title
|
|
|
I suspect that this doesn't hurt, because at the point you stop copying input to a file, you are done with the input, and I don't think there is any reason that the caller would ever continue reading it elsewhere. That said, the process is already terminated correctly in this case, which closes the streams too. I'm neutral on it unless this is theoretically solving a problem. |
|
Thanks @srowen |
|
I'm not sure that this solves a problem, and has some changes that look unrelated |
Closes apache#18522 Closes apache#17722 Closes apache#18879 Closes apache#18891 Closes apache#18806 Closes apache#18948 Closes apache#18949 Closes apache#19070 Closes apache#19039 Closes apache#19142 Closes apache#18515 Closes apache#19154 Closes apache#19162 Closes apache#19187
5 participants
What changes were proposed in this pull request?
Closes inputstream or outputstream and releases any system resources associated
with the stream.
How was this patch tested?
unit test