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

Use panic-safety helper functions from ffi-support #32

Closed
rfk opened this issue Jul 28, 2020 · 1 comment · Fixed by #221
Closed

Use panic-safety helper functions from ffi-support #32

rfk opened this issue Jul 28, 2020 · 1 comment · Fixed by #221
Assignees
@linabutler
Labels
blocks-nimbus Blocks Nimbus SDK on mobile

Comments

@rfk
Copy link
Collaborator

rfk commented Jul 28, 2020 •
edited by data-sync-user
Loading

Panics in the rust code are not currently handled very well, and I suspect would actually trigger undefined behaviour by propagating across the FFI boundary. We should use the support functions offered by ffi-support to ensure safe panic handling in all cases.

The most basic thing that could work:

  • All ffi functions should have an ExternError out parameter
  • The rust scaffolding for all standalone functions and object methods should wrap the call in call_with_output.

Object methods could instead rely on the call helpers of the HandleMap for the same effect, but I think it would be worth our while to catch and handle panics in argument conversion or other operations that might occur before we call into the HandleMap at all.

┆Issue is synchronized with this Jira Task
@rfk rfk mentioned this issue Jul 28, 2020
Merged
12 tasks
@rfk
Copy link
Collaborator Author

rfk commented Jul 28, 2020

For context, note that we currently panic if we receive unexpected data over the FFI, so we need to make that safe as well; ref https://github.com/rfk/uniffi-rs/pull/31#discussion_r460382079

@eoger eoger added the blocks-nimbus Blocks Nimbus SDK on mobile label Jul 29, 2020
@data-sync-user data-sync-user changed the title Use panic-safety helper functions from ffi-support SYNC-1556 ⁃ Use panic-safety helper functions from ffi-support Jul 30, 2020
@data-sync-user data-sync-user changed the title SYNC-1556 ⁃ Use panic-safety helper functions from ffi-support Use panic-safety helper functions from ffi-support Jul 31, 2020
linabutler added a commit that referenced this issue Aug 5, 2020
@linabutler
Ensure panic safety for all FFI methods.
cdbed39 
See #32.
@rfk rfk mentioned this issue Aug 8, 2020
Merged
linabutler added a commit that referenced this issue Aug 10, 2020
@linabutler
Ensure panic safety for all FFI methods.
698c7aa 
This commit changes all FFI functions to take an `out_err` param, so
that they're called using `call_with_result`. Any functions that don't
have a `[Throws]` annotation in the IDL will throw a generic
`InternalError` instead.

Closes #32.
linabutler added a commit that referenced this issue Aug 10, 2020
@linabutler
Ensure panic safety for all FFI methods.
959acba 
This commit changes all FFI functions to take an `out_err` param, so
that they're called using `call_with_result`. Any functions that don't
have a `[Throws]` annotation in the IDL will throw a generic
`InternalError` instead.

Closes #32.
@linabutler linabutler mentioned this issue Aug 10, 2020
Merged
linabutler added a commit that referenced this issue Aug 25, 2020
@linabutler
Ensure panic safety for all FFI methods.
2fd5b8c 
This commit changes all FFI functions to take an `out_err` param, so
that they're called using `call_with_result`. Any functions that don't
have a `[Throws]` annotation in the IDL will throw a generic
`InternalError` instead.

Closes #32.
linabutler added a commit that referenced this issue Aug 25, 2020
@linabutler
Ensure panic safety for all FFI methods.
f82e783 
This commit changes all FFI functions to take an `out_err` param, so
that they're called using `call_with_result`. Any functions that don't
have a `[Throws]` annotation in the IDL will throw a generic
`InternalError` instead.

Closes #32.
linabutler added a commit that referenced this issue Aug 26, 2020
@linabutler
Ensure panic safety for all FFI methods.
90ed61d 
This commit changes all FFI functions to take an `out_err` param, so
that they're called using `call_with_result`. Any functions that don't
have a `[Throws]` annotation in the IDL will throw a generic
`InternalError` instead.

Closes #32.
linabutler added a commit that referenced this issue Aug 26, 2020
@linabutler
Ensure panic safety for all FFI methods.
5077989 
This commit changes all FFI functions to take an `out_err` param, so
that they're called using `call_with_result`. Any functions that don't
have a `[Throws]` annotation in the IDL will throw a generic
`InternalError` instead.

Closes #32.
linabutler added a commit that referenced this issue Aug 27, 2020
@linabutler
Ensure panic safety for all FFI methods.
301f3e3 
This commit changes all FFI functions to take an `out_err` param, so
that they're called using `call_with_result`. Any functions that don't
have a `[Throws]` annotation in the IDL will throw a generic
`InternalError` instead.

Closes #32.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@linabutler linabutler

Labels
blocks-nimbus Blocks Nimbus SDK on mobile
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ensure panic safety for all FFI methods mozilla/uniffi-rs
3 participants
@rfk @eoger @linabutler