Skip to content

threads::diff: use separate git_repository objects#4754

Merged
ethomson merged 2 commits intomasterfrom
ethomson/threads
Aug 19, 2018
Merged

threads::diff: use separate git_repository objects#4754
ethomson merged 2 commits intomasterfrom
ethomson/threads

Conversation

@ethomson
Copy link
Copy Markdown
Member

@ethomson ethomson commented Aug 5, 2018

Our thread policies state that we cannot re-use the git_repository
across threads. Our tests cannot deviate from that.

Our thread policies state that we cannot re-use the `git_repository`
across threads.  Our tests cannot deviate from that.
@ethomson
Copy link
Copy Markdown
Member Author

ethomson commented Aug 5, 2018

Fixes #4753

@ethomson
Copy link
Copy Markdown
Member Author

ethomson commented Aug 5, 2018

I considered simply deleting these tests - it feels rather obvious that this should always succeed now that we're using multiple repository objects. But after more consideration, this does exercise the object cache, etc, and it seems reasonable to keep it.

@pks-t
Copy link
Copy Markdown
Member

pks-t commented Aug 6, 2018

Does it really? I thought the object cache was per repository.

@infinity0
Copy link
Copy Markdown

This improves the situation but doesn't get rid of the segfault completely, I posted another stack trace in #4753.

@carlosmn
Copy link
Copy Markdown
Member

carlosmn commented Aug 8, 2018

The object caches are per-repository but the packfiles and their caches are shared. I don't see how we would fail on ignore rules unless we're grabbing from a global cache of built-in rules or mismanaging the lifetime of these objects.

Our thread policies state that we cannot re-use the `git_repository`
across threads.  Our tests cannot deviate from that.

Courtesy of Ximin Luo, https://github.com/infinity0:

#4753 (comment)
@ethomson ethomson merged commit cada553 into master Aug 19, 2018
@ethomson
Copy link
Copy Markdown
Member Author

Thanks, @infinity0, I've added your change from #4753.

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