Skip to content

Multiple init #1890

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

Merged
merged 4 commits into from
Oct 5, 2013
Merged

Multiple init #1890

merged 4 commits into from
Oct 5, 2013

Conversation

ben
Copy link
Member

@ben ben commented Oct 3, 2013

As discovered in libgit2/libgit2sharp#517, initializing the threadsafe mechanisms multiple times doesn't seem to have any ill effects, but bad things would start to happen after the first call to git_threads_shutdown.

This PR converts the status variables to be something like a reference count, and the global threading primitives are only freed once the counter reaches zero.


int git_threads_init(void)
{
int error;

if (_tls_init)
if (git_atomic_inc(&git__n_inits) > 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this returns > 1, you still have to spin waiting on git__initialized to be > 0 so that later threads will wait for the first thread to finish initialization.

@ben
Copy link
Member Author

ben commented Oct 4, 2013

Okay, this last couple of commits should guard against this race:

  1. The last shutdown call is received, triggering teardown—
  2. And a wild init call appears, and tries to set up those same resources.

I kind of want to squash this all down when it's finalized. I hate to have points in our history where this specific API is so terribly broken.

@vmg
Copy link
Member

vmg commented Oct 4, 2013

I am a little concerned with the fact that this is over-engineered because @arrbee's initial code for deinit was also over-engineered.

Is the thread-safe and multi-shutdown API we have right now really necessary? I think one global init and one global shutdown should be the goal of all the people using the library. Can you think of use cases where this multiple initialization is a requirement?

@ben
Copy link
Member Author

ben commented Oct 4, 2013

Libgit2Sharp has one, actually. A server process (like IIS) can spin up multiple AppDomains in a short period of time, and the static constructor for Libgit2Sharp.NativeMethods (which calls git_threads_init) will be called more than once. None of the AppDomains have any access to each other's managed memory, so none of them can know if the init has been called before at that level – we have to do it in native code.

@ben
Copy link
Member Author

ben commented Oct 5, 2013

Squashed up, and Travis and Janky are happy. I think this is ready.

@vmg
Copy link
Member

vmg commented Oct 5, 2013

Threads are C R A Y. Thanks for tacking this, guise!

vmg pushed a commit that referenced this pull request Oct 5, 2013
@vmg vmg merged commit 711333e into development Oct 5, 2013
@ben ben deleted the multiple-init branch October 6, 2013 03:58
phatblat pushed a commit to phatblat/libgit2 that referenced this pull request Sep 13, 2014
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

Successfully merging this pull request may close these issues.

4 participants