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

Detect multiple versions of crates with global state, which can result in bugs. #2363

Closed
eddyb opened this issue Feb 6, 2016 · 7 comments
Closed

Comments

@eddyb
Copy link
Member

eddyb commented Feb 6, 2016

Generally, in the absence of global state, via static or linking to C libraries, multiple instances of the same crate should be harmless, and the same-version case is already getting deduplicated.
As thread-local support and lazy_static use static internally, they should get caught as well.

I'm not sure what the best way for detecting this is, short of using --pretty=expanded and searching for static and link attributes.

As far as I'm aware, this has caused issues in the past with the log crate, where env_logger would have a visible effect only for some crates and not others.

It could have worse consequences, if a crate is using some static flag to guard initialization/uses of a C library, and multiple versions of that crate use the same C library (e.g. system-wide OpenGL), potentially allowing data races and other memory unsafe behavior.

In those instances, it might be even better to have an option in the Cargo.toml of such crates to deny having multiple versions downstream, with the static & linkage detection as a lint suggesting that the option be enabled.

cc @whitequark @edef1c

@eddyb
Copy link
Member Author

eddyb commented Feb 6, 2016

This would probably be much simpler as an unstable lint plugin running in crater (or even a completely new dedicated crates.io pass), now that I think about it.

@alexcrichton
Copy link
Member

I suspect this would probably work much better as an opt-in rather than an auto-detection scenario, but definitely seems like a legitimate thing to worry about

@edef1c
Copy link

edef1c commented Feb 17, 2016

@eddyb It's worth noting that not all lazy/static/etc is relevant — for example, in libfringe, I cache the page size (with loose ordering), which isn't global state and can run multiple times just fine, since it's just an optimisation.

@eddyb
Copy link
Member Author

eddyb commented Feb 17, 2016

@edef1c In that case I would expect you to suppress a warning, if it ends up being a built-in lint.

@jan-hudec
Copy link

In https://github.com/rust-locale/rust-locale I intend to work around this issue by extracting the globals into a helper crate and have the main crate depend on any version of it ("*"), so even if multiple versions of locale get pulled in, they will both use the same latest version of the helper.

However, locale has the advantage that the global configuration can be relatively small and have simple interface. Not all crates can do that.

@jan-hudec
Copy link

jan-hudec commented Apr 28, 2016

Also there is another situation where multiple versions cause a problem: If crates A and B implement some trait provided by crate C, but they chose different versions of it, then the crate including them will not be able to pass types from both to functions from C requiring that trait.

This is also problem for locale, because it will define formatting traits (similar to Display), expect other crates to define them for their types and then accept such types in corresponding formatting functions.

This problem is less dangerous, but more serious, because when it happens, it breaks compilation.

@carols10cents
Copy link
Member

I'm going to close this issue, because I think the problems discussed can be solved with one of:

Please reopen if I'm wrong!

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

No branches or pull requests

5 participants