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

workaround fork for CRT #4473

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

Conversation

TingDaoK
Copy link

@TingDaoK TingDaoK commented Mar 13, 2025

Notes:

  • A prove of concept, not ready to merge yet.
  • Needs adding integration tests and polish the code
    • I guess boto3 try to avoid using awscrt directly, I guess we could have s3transfer to expose join_all_native_threads
    • suggestions are welcome, or feel free to starting new branch if it's easier.
  • It depends on Fork workaround awslabs/aws-crt-python#628, so CI will fail until we update the dependency.

Issue:

  • In CRT, we creates background threads, and fork is not safe with background threads at all.
  • From CRT perspective, it's basically impossible to track all threads and recreate them after fork. So, pushing the threads management to boto3 here.
  • As boto3 owns the CRT S3 client, which is the only CRT resource boto3 uses with background threads (I am not 100% percent sure, but looked into botocore, the CRT singer depends on static credentials provider, so it's not creating background threads, and it works on my reproduce code.)
  • So, boto3 can register a fork handler to clean up the CRT client and wait for threads to join.
  • From the forked process, new client will be created if needed, and it will be safe to use.
  • Notes: logger can create background threads, but it's not managed thread that will block the call, and it's also kind safe to be used (not lead to a crash at least)

@TingDaoK TingDaoK changed the title workaround fork workaround fork for CRT Mar 13, 2025
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.

1 participant