Skip to content

Simplified res closing and prevented creating/closing process OutputS…#427

Closed
tbw777 wants to merge 1 commit intoapache:masterfrom
tbw777:twr
Closed

Simplified res closing and prevented creating/closing process OutputS…#427
tbw777 wants to merge 1 commit intoapache:masterfrom
tbw777:twr

Conversation

@tbw777
Copy link
Copy Markdown

@tbw777 tbw777 commented Jan 31, 2023

…tream and ErrorStream.

There is no needed to make local variables as null and opening unused streams to close them later.

…tream and ErrorStream.

There is no needed to make local variables as null and opening unused streams to close them later.
@garydgregory
Copy link
Copy Markdown
Member

My only concern here is that the method will now fail if anything goes wrong closing streams. This is a change in behavior that we do not test apparently. This could be considered reasonable if other processes try to use the same resources but I am not 100% sure. What do others think?

@tbw777
Copy link
Copy Markdown
Author

tbw777 commented Feb 1, 2023

@garydgregory
The exception will also be thrown to the top because there is no corresponding catch as in the original version. Or do you mean something else?

@garydgregory
Copy link
Copy Markdown
Member

Hm, two behavior might be the same indeed. I'll look again in the AM.

@tbw777 tbw777 closed this by deleting the head repository Apr 6, 2023
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

Successfully merging this pull request may close these issues.

2 participants