-
Notifications
You must be signed in to change notification settings - Fork 83
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
MbedTLS Reference counted instead of lifetimes #128
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not done with the review, but let's start here
6e41bdc
to
2ce3aa1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0be7829
to
cd9eb8e
Compare
f741aeb
to
1715f09
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this only partially addresses #9, please update the PR description so it doesn't close the issue on merge.
3344b8d
to
241be71
Compare
Arc Changes: - Each Config/Context/... will hold Arcs towards items it holds pointers to. - This forces objects to live as long as needed, once no longer used they get destroyed by reference counting. This allows passing the objects to multiple threads without worrying about lifetime. I've also added notes why classes are Sync where used. Let me know if I missed any classes. Usage example of an intermediate mbed-hyper integration is at: - https://github.com/fortanix/rust-mbedtls/tree/acruceru/wip-mbed-hyper-v2/mbedtls-hyper/examples/integrations There I added a crate to wrap hyper - similar to native-tls. (that will be moved to native-tls layer soon) That crate can be considered an integration test that I will raise a separate PR for.
65ea801
to
afecf09
Compare
bors try |
tryBuild succeeded: |
bors r+ |
128: MbedTLS Reference counted instead of lifetimes r=jethrogb a=AdrianCX Moving from referene counting allows simpler move to native-tls / hyper. Arc Changes: - Each Config/Context/... will hold Arcs towards items it holds pointers to. - This forces objects to live as long as needed, once no longer used they get destroyed by reference counting. This allows passing the objects to multiple threads without worrying about lifetime. I've also added notes why classes are Sync where used. Let me know if I missed any classes. Usage example of an intermediate mbed-hyper integration is at: - https://github.com/fortanix/rust-mbedtls/tree/acruceru/wip-mbed-hyper-v2/mbedtls-hyper/examples/integrations There I added a crate to wrap hyper - similar to native-tls. (that will be moved to native-tls layer soon) That crate can be considered an integration test that I will raise a separate PR for. Edit: Changes after initial review: - Added forward_mbedtls_calloc / forward_mbedtls_free functions so we can pass certificates to and from mbedtls without allocator mismatches/corruptions. - Switched to MbedtlsList<Certificate> and Certificate. A MbedtlsBox is pending for this PR as well. - Fixed most comments. Still pending: - Update define! macros - Add MbedtlsBox<Certificate> Fixes #1 Partial progress on #3 Fixes #4 Fixes #8 Partially addresses #9 Co-authored-by: Adrian Cruceru <[email protected]>
Timed out. |
bors r+ |
Build succeeded: |
Gets rid of synthetic_error, and makes the various send_* methods return `Result<Response, Error>`. Introduces a new error type "HTTP", which represents an error due to status codes 4xx or 5xx. The HTTP error type contains a boxed Response, so users can read the actual response if they want. Adds an `error_for_status` setting to disable the functionality of treating 4xx and 5xx as errors. Adds .unwrap() to a lot of tests. Fixes fortanix#128.
Moving from referene counting allows simpler move to native-tls / hyper.
Arc Changes:
This allows passing the objects to multiple threads without worrying about lifetime.
I've also added notes why classes are Sync where used. Let me know if I missed any classes.
Usage example of an intermediate mbed-hyper integration is at:
There I added a crate to wrap hyper - similar to native-tls. (that will be moved to native-tls layer soon)
That crate can be considered an integration test that I will raise a separate PR for.
Edit:
Changes after initial review:
Still pending:
Fixes #1
Partial progress on #3
Fixes #4
Fixes #8
Partially addresses #9