acidfs icon indicating copy to clipboard operation
acidfs copied to clipboard

git segfault when concurrently writing conflicting strings with leading newline

Open epistemery opened this issue 9 years ago • 5 comments

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?

epistemery avatar Feb 15 '16 18:02 epistemery

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 avatar Feb 15 '16 19:02 chrisrossi

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.

chrisrossi avatar Feb 15 '16 20:02 chrisrossi

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 [email protected]. Not visible on gmane yet, I'll update this post.

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

epistemery avatar Feb 15 '16 21:02 epistemery

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.

epistemery avatar Feb 19 '16 12:02 epistemery

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 [email protected] 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 https://github.com/Pylons/acidfs/issues/3#issuecomment-186204667.

chrisrossi avatar Feb 19 '16 15:02 chrisrossi