Skip to content
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

git segfault when concurrently writing conflicting strings with leading newline #3

Open
epistemery opened this issue Feb 15, 2016 · 5 comments

Comments

@epistemery
Copy link

I've experienced a strange git sigfault error when I tried to provoke a conflict while writing strings from two concurrent threads. The Problem seems to be a newline (\n) as first character. Strangely it only occurs when there was a previous commit in the repo.

Tested with Python 2.7, Python 3.4, git 1.9.1 and git 2.7.1. While Python 3.4 throws an exception as subprocess.Popen exits with non-zero return code, Python 2.7 explicitly tells me about a malloc memory corruption:

*** Error in 'git': malloc(): memory corruption (fast): 0x0000000001c97700 ***

Test suite:

https://gist.github.com/epistemery/90340019b6d2b4853355

As I am not very thread-experienced I hope the problem is not related to the test setup. I can't see why it should be, but who knows...

It looks like a git problem to me, but I can't tell where to start investigating, because I'm not familiar with AcidFS internals. All I can tell so far is that the code starting at line 854 never gets called when there is a leading newline in one of the conflicting merges.

Any Ideas where to start?

@chrisrossi
Copy link
Member

It's a bug in git. I was able to reproduce with a shell script independently of AcidFS:

https://gist.github.com/chrisrossi/f09c8bed70b364f9f12e

Maybe you can show that to the git folks.

HTH.

@chrisrossi
Copy link
Member

FWIW, even with the malloc error, the output of merge-tree is still correct and AcidFS still works as expected. You just get a nasty-gram to stderr.

@epistemery
Copy link
Author

Thank you! I wasn't aware that the git test case would be so simple. Sorry for the extra work.. I guess the fact that that the code starting at line 854 in __init__.py is not visited made me think that it had to do with some specific merge attempt...

I sent a bug report to git@vger.kernel.org. Not visible on gmane yet, I'll update this post.

EDIT: http://comments.gmane.org/gmane.comp.version-control.git/286252

@epistemery
Copy link
Author

Bad news. It looks like merge-tree is something like a proof-of-concept, follows a different merge-logic than git merge and probably has more design/implementation flaws yet to be discovered...

Jeff King suggests an alternative for a tree-level merge:

If you just care about doing the tree-level merge
without content-level merging inside blobs, you can do it in the index
like:

export GIT_INDEX_FILE=temp.index
base=$(git merge-base $ours $theirs)
git read-tree -i -m --aggressive $base $ours $theirs

If you want to do content-level resolving on top of that, you can do it
with:

git merge-index git-merge-one-file -a

though it will need a temp directory to write out conflicts and
resolved content.

I can't tell whether this is useful for acidfs, but maybe the merge logic code should be changed to rely on more robust git tools.

Anyway, there now is a patch for the segfault issue.

@chrisrossi
Copy link
Member

Interesting. I'll take a look when I get a chance. Thanks! If the output
format is the same or very similar, it shouldn't be too hard to switch.

Chris

On Fri, Feb 19, 2016 at 7:59 AM, epistemery notifications@github.com
wrote:

Bad news. It looks like merge-tree is something like a proof-of-concept,
follows a different merge-logic than git merge and probably has more
design/implementation flaws yet to be discovered...

Jeff King suggests
http://permalink.gmane.org/gmane.comp.version-control.git/286429 an
alternative for a tree-level merge:

If you just care about doing the tree-level merge
without content-level merging inside blobs, you can do it in the index
like:

export GIT_INDEX_FILE=temp.index
base=$(git merge-base $ours $theirs)
git read-tree -i -m --aggressive $base $ours $theirs

If you want to do content-level resolving on top of that, you can do it
with:

git merge-index git-merge-one-file -a

though it will need a temp directory to write out conflicts and
resolved content.

I can't tell whether this is useful for acidfs, but maybe the merge logic
code should be changed to rely on more robust git tools.

Anyway, there now is a patch
http://permalink.gmane.org/gmane.comp.version-control.git/286298 for
the segfault issue.


Reply to this email directly or view it on GitHub
#3 (comment).

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

No branches or pull requests

2 participants