git2-rs icon indicating copy to clipboard operation
git2-rs copied to clipboard

Implement Send for libgit2 structs

Open Aaron1011 opened this issue 8 years ago • 5 comments

According to the libgit2 docs, it it safe to use objects on a thread other than the one they are created on (though there are some special considerations for older versions of OpenSSL). As such, it should be safe to implement Send (but not Sync) for all core libgit2 structs (Commit, Author, etc).

Aaron1011 avatar Mar 23 '17 19:03 Aaron1011

Yeah I definitely want to make sure structs are send/sync when they need to be. Many objects aren't, though, so I just wanted to be careful and manually verify the implementations when doing so. PRs are of course welcome!

alexcrichton avatar Mar 23 '17 23:03 alexcrichton

Would love to see this happen too. For reference, here's a permalink to the relevant docs since the one in the first comment is now broken:

https://github.com/libgit2/libgit2/blob/68a3c0b1244687ce07c3d8605e875485d86b05ff/docs/threading.md

Relevant excerpt:

Use an object from a single thread at a time. Most data structures do not guard against concurrent access themselves. This is because they are rarely used in isolation and it makes more sense to synchronize access via a larger lock or similar mechanism.

There are some objects which are read-only/immutable and are thus safe to share across threads, such as references and configuration snapshots.

So it sounds like there's room for marking structs as Send and some of them as Sync also.

jlebon avatar Sep 09 '18 03:09 jlebon

+1

refaelsh avatar Jun 13 '21 19:06 refaelsh

I would like to store a global immutable reference to a GitConfig (I was thinking I could use a lazy_static for this.) Can I check my understanding -- is this ticket being open, and the discussion here, telling me that currently that's not possible? But it is a reasonable desire in principle, right?

(The error I get is *mut libgit2_sys::git_config cannot be shared between threads safely. I have not specifically asked for it to be mutable; I'm using git2::Config::open_default() and git2::Repository::discover(dir).unwrap().config() to obtain the config.)

dandavison avatar Nov 22 '21 17:11 dandavison

One good reason to implement at least Send for the types is to be able to pass them and return them from tokio::task::spawn_blocking(), which would run the blocking operations in this crate on dedicated I/O threads. @alexcrichton how would you suggest to integrate this library with async runtime then? One possible workaround I see is using block_in_place, but it blocks the current thread, where it should better be running on a dedicated one...

Veetaha avatar Mar 23 '22 11:03 Veetaha