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

Fixes sgx main loop not handling signals properly #282

Merged
merged 2 commits into from
Jan 24, 2025

Conversation

italo-sampaio
Copy link
Collaborator

@italo-sampaio italo-sampaio commented Jan 24, 2025

Main loop blocks while waiting for client data. For this reason, we can't just check for the stop condition synchronously. In any case, we still need to ensure that the finalise logic is only called once.

This PR reverts commit 1ec2d35 and introduces a global counter to ensure finalise_with is only called once.

Copy link

github-actions bot commented Jan 24, 2025

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Files

@italo-sampaio italo-sampaio force-pushed the fix/unsafe-signal-handling branch 2 times, most recently from cc0bf99 to 1f1b806 Compare January 24, 2025 12:43
Introduces a global counter to ensure finalise_with is only called once.
@italo-sampaio italo-sampaio force-pushed the fix/unsafe-signal-handling branch from 1f1b806 to c569c92 Compare January 24, 2025 13:50
@italo-sampaio italo-sampaio marked this pull request as ready for review January 24, 2025 13:50
Copy link
Collaborator

@amendelzon amendelzon left a comment

Choose a reason for hiding this comment

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

LGTM

@italo-sampaio italo-sampaio merged commit 8553680 into master Jan 24, 2025
8 checks passed
@italo-sampaio italo-sampaio deleted the fix/unsafe-signal-handling branch January 24, 2025 17:15
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