make the library thread-safe about adblock-rust HOT 12 CLOSED

This is also a problem for python-adblock. Upgrading PyO3 from 0.10 to 0.11 makes the build fail due to thread-safety issues. See https://github.com/ArniDagur/python-adblock/actions/runs/151271594.

from adblock-rust.

I don't have a good way to test this, as adblock-rust isn't really used in a multithreaded context in Brave. However, I would be happy to look at PRs for this.

from adblock-rust.

It's worth noting that the library used to be thread safe, but this was changed in 19ac71a for apparent performance reasons.

@AndriusA Would you be fine with reverting the aforementioned commit?

from adblock-rust.

@antonok-edm What do you think?

from adblock-rust.

@ArniDagur it's actually been discussed before - see #101 (comment). Ultimately it seems like the gains from thread-unsafety are more hassle than they're worth, so I would support rolling that change back.

from adblock-rust.

I'll make a PR.

EDIT: #130

from adblock-rust.

Should be fixed by #130. @wavenator if this doesn't work for you feel free to follow up.

from adblock-rust.

Thanks a lot for all the effort!

from adblock-rust.

This package is still not thread safe even with the rollback. Related discussion: https://stackoverflow.com/questions/63761408/rust-rocket-pass-variable-to-endpoints

from adblock-rust.

Yeah, you have to disable the object-pooling feature. That worked for me at least. Sorry for the confusion.

from adblock-rust.

Yeah, you have to disable the object-pooling feature. That worked for me at least. Sorry for the confusion.

Can you tell me how to disable object-pooling? I never touched Rust before and I'm stuck here. Thank you.

from adblock-rust.

@ciokan When you add a dependency in your Cargo.toml file, rather than just giving a version number you can specify more information, including a list of "features". Here, you'll also want to disable default features, since object-pooling is on by default. This crate's supported features are all listed under [features] in this repo's Cargo.toml.

In your case you could add a new section that might look something like this:

[dependencies.adblock]
version = "0.3.2"
default-features = false
features = ["full-regex-handling", "embedded-domain-resolver"]

Or if you prefer, you can equivalently specify the whole dependency inline under the usual [dependencies] section like this:

[dependencies]
adblock = { version = "0.3.2", default-features = false, features = ["full-regex-handling", "embedded-domain-resolver"] }

from adblock-rust.