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.

Sign up for GitHub

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

Jump to bottom

issue-2409: CRT logging thread runs beyond usable lifetime of CRT and dependents #2410

Closed
wants to merge 1 commit into from
Closed

issue-2409: CRT logging thread runs beyond usable lifetime of CRT and dependents #2410

wants to merge 1 commit into from

Conversation

jeking3
Copy link
Contributor

@jeking3 jeking3 commented Mar 29, 2023

The CRT logging thread was outliving the CRT and dependents, including aws-c-common. If one is using CUSTOM_MEMORY_MANAGEMENT then the aws-c-common allocator will be NULL, but the logging thread is still running, and may try to allocate, causing an assert.

In general, shutdown should be the reverse sequence of init to avoid situations like this. #1996 may have fixed something, but the fix is in the wrong place.

Issue #, if available:

This fixes #2409

Description of changes:

  1. Restore the correct shutdown sequence, an inverse of the init sequence.
  2. Clean up cJSON for good measure, properly.
  3. Add a test that proves it is okay to Init and Shutdown many times in the same executable run.

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jeking3
issue-2409: Fix incorrect shutdown call order
253368f 
The CRT logging thread was outliving the CRT and dependents,
including aws-c-common.  If one is using CUSTOM_MEMORY_MANAGEMENT
then the aws-c-common allocator will be NULL, but the logging
thread is still running, and may try to allocate, causing an assert.

In general, shutdown should be the reverse sequence of init to
avoid situations like this.  aws#1996 may have fixed something, but
the fix is in the wrong place.
@jmklix jmklix added the needs-review This issue or pull request needs review from a core team member. label Mar 30, 2023
@jeking3
Copy link
Contributor Author

jeking3 commented Mar 31, 2023

This is not a valid fix for the issue; it re-introduces the instability described in #1995 and fixed in #1996. We need another way to fix this for CUSTOM_MEMORY_MANAGEMENT.

@jeking3 jeking3 closed this Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
needs-review This issue or pull request needs review from a core team member.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Loggers run beyond the usable lifetime of CRT
2 participants
@jeking3 @jmklix