fix(ffi): Fix FFI usage on distros using SELinux #15307
Conversation
|
|
ext/ffi/lib.rs
Outdated
| @@ -780,8 779,16 @@ fn make_sync_fn<'s>( | |||
|
|
|||
| #[cfg(not(target_os = "windows"))] | |||
| let mut fast_allocations: Option<*mut ()> = None; | |||
|
|
|||
| #[cfg(target_os = "linux")] | |||
| let selinux_is_enabled = | |||
There was a problem hiding this comment.
Presumably this could also be outside the function call entirely?
There was a problem hiding this comment.
You want me to use thread_local! or lazy_static ?
Edit: Now using once_cell
Put back the little space between `mod` and `use`
|
I'm using OnceCell because it will soon be integrated inside the rust standard library |
Co-authored-by: Aapo Alasuutari <[email protected]>
ext/ffi/lib.rs
Outdated
| @@ -766,6 768,24 @@ fn is_fast_api_rv(rv: NativeType) -> bool { | |||
| ) | |||
| } | |||
|
|
|||
| #[cfg(target_os = "linux")] | |||
There was a problem hiding this comment.
Let's add a todo comment: // TODO(@littledivy): Don't use normal heap with mprotect PROT_EXEC for executable code to support SELinux. to somehow make this optimization work in the future
There was a problem hiding this comment.
What sort of way would there be to make this work without normal heap mprotect?
There was a problem hiding this comment.
Ah, okay. In that case the dynasmrt will then handle this as it uses mmap. Nice!
ext/ffi/lib.rs
Outdated
| #[cfg(target_os = "linux")] | ||
| let fastapi_is_available = *is_fastapi_available(); | ||
| #[cfg(all(not(target_os = "linux"), not(target_os = "windows")))] | ||
| let fastapi_is_available = true; |
There was a problem hiding this comment.
It would be better if these conditional compilations were inside the is_fastapi_available() rather than conditionally defining this function...
There was a problem hiding this comment.
I changed it, do you like it better that way ?
# Conflicts: # ext/ffi/lib.rs
|
|
|
It seems not to be as easy as we want it to be, we might either use back libloading or wait for #15305 |
|
Since this PR: #15305 would make this problem inexistent with the removal of TinyCC I will close this PR and support the other one. |
This is related to issue #15306