rust-lang / git2-rs

libgit2 bindings for Rust
https://docs.rs/git2
Apache License 2.0
1.65k stars 381 forks source link

Implement Send for all simple git types #924

Closed orf closed 11 months ago

orf commented 1 year ago

This implements Send for all simple libgit2-sys types. According to https://github.com/libgit2/libgit2/blob/main/docs/threading.md this is fine - objects can be sent between threads.

Refs https://github.com/rust-lang/git2-rs/issues/194

mitermayer commented 1 year ago

Are there any plans on merging this any time soon ?

orf commented 1 year ago

I’m not sure the best way to test this, but I’ve used this branch on an extensively multi-threaded application that opens, utilises and exchanges hundreds of repositories between threads without issue.

That’s definitely a bit “source: trust me bro” but it does seem perfectly fine.

I did actually encounter one issue - multiple threads writing to a single odb instance in parallel using the odb writer interface seems to encounter race conditions and suffers from incomplete objects being written.

That’s one of the few objects explicitly and already marked as “safe to use from multiple threads”.

mitermayer commented 1 year ago

I really wish this PR could be merged, or if in the docs was some clear documentation with workarounds

ngalaiko commented 10 months ago

@orf did you find some issues with that, or just closed due to testing issues?

orf commented 10 months ago

Oh, no, I just deleted a bunch of my forks. It works fine

Piturnah commented 9 months ago

Any chance of this being reopened?

clouds56 commented 4 months ago

I did actually encounter one issue - multiple threads writing to a single odb instance in parallel using the odb writer interface seems to encounter race conditions and suffers from incomplete objects being written.

That’s one of the few objects explicitly and already marked as “safe to use from multiple threads”.

According to this PR https://github.com/libgit2/libgit2/pull/5595, I think odb of libgit2 should be "thread-safe". If it doesn't, I think it's a bug in libgit2, not ours.

BTW, although we could use odb in multithread, it could still failed to call due to "locked" by another thread, have you check the returning error code?

I think we should mark them "Send" now and when we encounter some bug in the future, we could help open an issue in libgit2.

joshbenz commented 3 months ago

+1