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

Compile error in Rust 1.36.0 #543

Closed
phiresky opened this issue Jul 8, 2019 · 15 comments
Closed

Compile error in Rust 1.36.0 #543

phiresky opened this issue Jul 8, 2019 · 15 comments
Labels

Comments

@phiresky
Copy link
Contributor

phiresky commented Jul 8, 2019

rusqlite fails to compile on Rust 1.36.0:

   Compiling rusqlite v0.19.0
error[E0277]: `*const libsqlite3_sys::sqlite3_module` cannot be sent between threads safely
  --> /home/tehdog/.cargo/registry/src/github.com-1ecc6299db9ec823/rusqlite-0.19.0/src/vtab/series.rs:21:1
   |
21 | / lazy_static! {
22 | |     static ref SERIES_MODULE: Module<SeriesTab> = eponymous_only_module::<SeriesTab>(1);
23 | | }
   | |_^ `*const libsqlite3_sys::sqlite3_module` cannot be sent between threads safely
   |
   = help: within `vtab::Module<vtab::series::SeriesTab>`, the trait `std::marker::Send` is not implemented for `*const libsqlite3_sys::sqlite3_module`
   = note: required because it appears within the type `libsqlite3_sys::sqlite3_vtab`
   = note: required because it appears within the type `vtab::series::SeriesTab`
   = note: required because it appears within the type `std::marker::PhantomData<vtab::series::SeriesTab>`
   = note: required because it appears within the type `vtab::Module<vtab::series::SeriesTab>`
   = note: required because of the requirements on the impl of `std::marker::Sync` for `spin::once::Once<vtab::Module<vtab::series::SeriesTab>>`
   = note: required because it appears within the type `lazy_static::lazy::Lazy<vtab::Module<vtab::series::SeriesTab>>`
   = note: shared static variables must have a type that implements `Sync`
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

error[E0277]: `*mut i8` cannot be sent between threads safely
  --> /home/tehdog/.cargo/registry/src/github.com-1ecc6299db9ec823/rusqlite-0.19.0/src/vtab/series.rs:21:1
   |
21 | / lazy_static! {
22 | |     static ref SERIES_MODULE: Module<SeriesTab> = eponymous_only_module::<SeriesTab>(1);
23 | | }
   | |_^ `*mut i8` cannot be sent between threads safely
   |
   = help: within `vtab::Module<vtab::series::SeriesTab>`, the trait `std::marker::Send` is not implemented for `*mut i8`
   = note: required because it appears within the type `libsqlite3_sys::sqlite3_vtab`
   = note: required because it appears within the type `vtab::series::SeriesTab`
   = note: required because it appears within the type `std::marker::PhantomData<vtab::series::SeriesTab>`
   = note: required because it appears within the type `vtab::Module<vtab::series::SeriesTab>`
   = note: required because of the requirements on the impl of `std::marker::Sync` for `spin::once::Once<vtab::Module<vtab::series::SeriesTab>>`
   = note: required because it appears within the type `lazy_static::lazy::Lazy<vtab::Module<vtab::series::SeriesTab>>`
   = note: shared static variables must have a type that implements `Sync`
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0277`.
error: Could not compile `rusqlite`.

To learn more, run the command again with --verbose.

Seems to happen for both 0.19.0 and 0.18.0
Via this error report: phiresky/ripgrep-all#22

@gwenn
Copy link
Collaborator

gwenn commented Jul 8, 2019

In the original C version, seriesModule is static.
So I guess we can fix this by telling Rust that sqlite3_module is Send and Sync.
I will try to fix and add a test.
Thanks.

@gwenn
Copy link
Collaborator

gwenn commented Jul 8, 2019

I cannot reproduce the error...
But https://docs.rs/lazy_static/*/lazy_static/:

Any type in them needs to fulfill the Sync trait.

@phiresky
Copy link
Contributor Author

phiresky commented Jul 8, 2019

Uhh, could also be an interaction with some other library? Can you try and see if you get the error if you do cargo install ripgrep_all?

@gwenn
Copy link
Collaborator

gwenn commented Jul 8, 2019

I tried

$ git clone https://github.com/phiresky/ripgrep-all.git
$ cd ripgrep-all/
$ cargo +stable check 

And your Travis build seems ok.
And https://github.com/jgallagher/rusqlite/blob/master/src/vtab/mod.rs#L70:

unsafe impl<T: VTab> Sync for Module<T> {}

Module is Sync !

@phiresky
Copy link
Contributor Author

phiresky commented Jul 8, 2019

to be honest, I don't know enough about rust to really understand what is happening..

cargo +stable check only started failing for me when I ran cargo upgrade and let it regenerate the lock file (try branch 1.36-weird)

@phiresky
Copy link
Contributor Author

phiresky commented Jul 8, 2019

Oh yeah I should also mention I have vtab and bundled active

@Legogris
Copy link

Legogris commented Jul 9, 2019

Getting the same when running cargo install riprep_all with Cargo 1.35.0 on a CentOS 7 install with no prior Rust exposure.

@thomcc
Copy link
Member

thomcc commented Jul 9, 2019

Yeah, I can also repro, but building from the ripgrep-all repo works fine. Wonder if it's a rustc bug or something.

@gwenn
Copy link
Collaborator

gwenn commented Jul 10, 2019

When we say:

unsafe impl<T: VTab> Sync for Module<T> {}

Rust wants that T is Send ?

@gwenn
Copy link
Collaborator

gwenn commented Jul 10, 2019

Workaround:
It appears that you are only using the vtab feature for:
rusqlite::vtab::escape_double_quote(&table)
And you are not using it correctly:
"select * from {}",
should be:
"select * from \"{}\"",

Ideally, we would like to use sqlite3_mprintf and "%w" but variadic args are not supported.

I am going to introduce a dedicated series feature.
So you should not have the compile error anymore.

@phiresky
Copy link
Contributor Author

oh - guess that means an sqlite database do an sql injection on itself with my code 😆

thanks for the effort!

@Br1ght0ne
Copy link

@gwenn Can we get a release incorporating this fix? Thanks!

@gwenn
Copy link
Collaborator

gwenn commented Jul 27, 2019

@filalex77 I would like to merge #549 before.

@Br1ght0ne
Copy link

@gwenn Sure, thanks for the reply.

@gwenn
Copy link
Collaborator

gwenn commented Jul 27, 2019

Should be fixed by release 0.20.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants