Skip to content

Ignore munlock failures #2804

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

Merged
merged 2 commits into from
May 5, 2021
Merged

Ignore munlock failures #2804

merged 2 commits into from
May 5, 2021

Conversation

SaswatPadhi
Copy link
Contributor

@SaswatPadhi SaswatPadhi commented May 5, 2021

Resolved issues:

None.

Related to: #2803.
fix: #2803

Description of changes:

Instead of failing on munlock, we now do a best-effort munlock.

Call-outs:

We might want to refactor this part later on, and properly handle munlock failures. I left #2803 open for discussion.

Testing:

N/A

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

@SaswatPadhi SaswatPadhi marked this pull request as draft May 5, 2021 20:50
@SaswatPadhi SaswatPadhi self-assigned this May 5, 2021
@SaswatPadhi SaswatPadhi changed the title fix Ignore munlock failures May 5, 2021
@SaswatPadhi SaswatPadhi marked this pull request as ready for review May 5, 2021 20:59
@lrstewart lrstewart requested a review from SalusaSecondus May 5, 2021 22:22
@feliperodri feliperodri merged commit d69cdab into aws:main May 5, 2021
free(ptr);
POSIX_ENSURE(munlock_rc == 0, S2N_ERR_MUNLOCK);
Copy link
Contributor

Choose a reason for hiding this comment

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

should we also remove S2N_ERR_MUNLOCK from s2n_errno.c since this doesnt exist anymore?

Copy link
Contributor

@toidiu toidiu May 6, 2021

Choose a reason for hiding this comment

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

thinking about this more, removing S2N_ERR_MUNLOCK might be a breaking api change so possibly no good

@SaswatPadhi SaswatPadhi deleted the munlock_fix branch May 6, 2021 20:36
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.

Handle munlock errors consistently
4 participants