Skip to content

Demonstrate file::GetContents / SetContents memory leak #4009

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

Closed
wants to merge 1 commit into from

Conversation

pjh
Copy link
Contributor

@pjh pjh commented Dec 2, 2023

To reproduce the memory leak cherry-pick this PR, ensure you have valgrind installed and then:

bazel build //ortools/base:hello_file
valgrind --leak-check=full ./bazel-bin/ortools/base/hello_file &> leak.out
bazel build --cxxopt=-DFIX_MEMORY_LEAK //ortools/base:hello_file
valgrind --leak-check=full ./bazel-bin/ortools/base/hello_file &> no-leak.out

Here's the output I see: leak.out, no-leak.out

valgrind reports "definitely lost: 48 bytes in 2 blocks" for or-tools at head, but "definitely lost: 0 bytes in 0 blocks" when the File is deleted after closing it.

bazel build //ortools/base:hello_file
./bazel-bin/ortools/base/hello_file
valgrind --leak-check=full ./bazel-bin/ortools/base/hello_file &> leak.out
bazel build --cxxopt=-DFIX_MEMORY_LEAK //ortools/base:hello_file
./bazel-bin/ortools/base/hello_file
valgrind --leak-check=full ./bazel-bin/ortools/base/hello_file &> no-leak.out
@pjh pjh force-pushed the demonstrate-memory-leak branch from 0318013 to df579a7 Compare December 2, 2023 01:06
@pjh pjh changed the title Demonstrate file::Open memory leak Demonstrate file::GetContents / SetContents memory leak Dec 2, 2023
@flomnes
Copy link
Contributor

flomnes commented Mar 22, 2024

Since this issue has been solved by

I think it's safe to close this ticket.

Personal opinion: returning std::unique_ptr<File> would be safer ie there would be no room for developers to forget resource liberation.

@pjh pjh closed this Mar 22, 2024
@Mizux Mizux self-assigned this Mar 25, 2024
@Mizux Mizux added this to the v10.0 milestone Mar 25, 2024
@Mizux Mizux added Bug Lang: C++ Native implementation issue labels Mar 25, 2024
@Mizux Mizux modified the milestones: v10.0, v9.10 Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Lang: C++ Native implementation issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants