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 reader, stream, and import leaks in CLI #301

Merged
merged 3 commits into from
Jul 14, 2022

Conversation

nirosys
Copy link
Contributor

@nirosys nirosys commented Jul 9, 2022

Issue #, if available: None

Description of changes:
Minor fixes to release memory in two spots within cli.cpp.

The first was when opening a writer via ion_cli_open_writer with imports, the has_imports flag would not be set on the writer context, resulting in leaked imports from ion_event_writer_close not closing the imports.

The second is a stream and reader leak when cleaning up the reader_contexts in ion_cli_command_compare_standard.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

ion_cli_open_writer was not setting the writer context's has_imports to
true after adding shared imports, which resulted in the imports not
being closed when the writer was closed.
Copy link
Contributor

@cheqianh cheqianh left a comment

Choose a reason for hiding this comment

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

LGTM!

@nirosys nirosys marked this pull request as ready for review July 14, 2022 00:13
@nirosys nirosys merged commit 400be4f into amazon-ion:master Jul 14, 2022
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