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

Fix resource leak bug #5251

Merged
merged 2 commits into from
Jul 5, 2023
Merged

Fix resource leak bug #5251

merged 2 commits into from
Jul 5, 2023

Conversation

Alfusainey
Copy link
Contributor

Description (required)

This patch fixes a potential resource leak bug in the event that an exception is thrown during the copy operation

Copy link
Member

@sivaraam sivaraam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! For the sake of reference, could you kindly clarify how you came across this resource leakage issue ? 🤔

@Alfusainey
Copy link
Contributor Author

Thanks for your contribution! For the sake of reference, could you kindly clarify how you came across this resource leakage issue ? 🤔

we investigated open-source projects to see if the projects contain code that is similar to (or reused from) Stack Overflow and whether the code on Stack Overflow underwent any bug or security fixes. We found that the code in this method is somewhat similar to the one in the first version of this answer. The issue was subsequently fixed in the second version, however, the fix is not reflected in this method.

So this PR is a way of notifying about the issue

Signed-off-by: Alfusainey Jallow <alf.jallow@gmail.com>
Copy link
Member

@sivaraam sivaraam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we investigated open-source projects to see if the projects contain code that is similar to (or reused from) Stack Overflow and whether the code on Stack Overflow underwent any bug or security fixes. We found that the code in this method is somewhat similar to the one in the first version of this answer. The issue was subsequently fixed in the second version, however, the fix is not reflected in this method.

So this PR is a way of notifying about the issue

Great. Thanks for the helpful work! Much appreciated 🙂

@Alfusainey
Copy link
Contributor Author

@sivaraam I updated the PR to incorporate your comments. Can you please take another look?

Signed-off-by: Alfusainey Jallow <alf.jallow@gmail.com>
@sivaraam
Copy link
Member

sivaraam commented Jul 5, 2023

Looks great. Thank your for your contribution!

@sivaraam sivaraam merged commit 368e1c7 into commons-app:master Jul 5, 2023
1 check passed
@Alfusainey Alfusainey deleted the bug-fix branch July 6, 2023 07:43
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.

None yet

2 participants