Fixed resource leak in sootup.java.bytecode.frontend module#1099
Fixed resource leak in sootup.java.bytecode.frontend module#1099swissiety merged 6 commits intosoot-oss:developfrom
Conversation
swissiety
left a comment
swissiety
left a comment
There was a problem hiding this comment.
thx for your contribution!
| .collect(Collectors.toList()); | ||
| return javaSootClassSources.stream(); |
There was a problem hiding this comment.
please don' t .collect() and .stream() again (return stream directly)
There was a problem hiding this comment.
Hi @swissiety since streams are lazy, if we don't collect, we cannot safely close the Stream returned by Files.walk within this method (since the filter and map operations won't have been executed yet). The collect forces those operations to execute, so then the stream can be closed. If you prefer not to collect and then stream again, an alternative would be to require all callers of this method to close the returned stream. Would that be preferable?
There was a problem hiding this comment.
thx @msridhar for pointing to that situation! As this is a protected method and its calling contexts are collecting the Stream again i would prefer to close the Stream in the two calling method(s) due to have less allocations
There was a problem hiding this comment.
@Bryce7832 could you update the PR to get rid of the collect to a list, and instead use try-with-resources at both callers of this method?
swissiety
force-pushed
the
bug/resourceleak_bytecode
branch
from
3f25697 to
5b55176
Compare
swissiety
force-pushed
the
bug/resourceleak_bytecode
branch
from
5b55176 to
a7bb4f8
Compare
3 participants
Fixed leak using try-with-resources. However, changed the method to return a
Listand do thecollectoperation inside the method to avoid try-with-resources closing the stream before its last use. This ensures the resources to be cleaned up. Fixes #1098