Am 4/19/2012 8:16, schrieb Johannes Sixt:
> Am 4/18/2012 21:53, schrieb Ramsay Jones:
>> Johannes Sixt wrote:
>>> Am 4/16/2012 8:44, schrieb Junio C Hamano:
>>>> * nd/threaded-index-pack (2012-04-11) 3 commits
>>>> - index-pack: support multithreaded delta resolving
>>>> - index-pack: split second pass obj handling into own function
>>>> - compat/win32/pthread.h: Add an pthread_key_delete() implementation
>>>
>>> With this series, t9300.92 (fast-import, Q: verify pack) consistently
>>> fails for me on Windows.
>>>
>>> I'll have to see when I can dig deeper into this topic...
>>
>> Hmm, this works just fine for me.
>
> It's a Heisenbug. I see different failure modes:
>
> error: inflate: data stream error (incorrect header check)
> fatal: serious inflate inconsistency
>
> fatal: premature end of pack file, 79 bytes missing
>
> fatal: premature end of pack file, 72 bytes missing
>
> (and I even saw successful runs). It's always the same pack,
> pack-54fa663f5ec35*.pack. But running index-pack --verify manually does
> not fail. The packs look good because the installed index-pack (which was
> built without this topic) does not report an error, either.
Here's one backtrace:
#0 error (err=0x55f6e6 "inflate: %s (%s)") at usage.c:122
#1 0x004ba96d in git_inflate (strm=0x12afdb8, flush=0) at zlib.c:144
#2 0x00434823 in get_data_from_pack (obj=0xd82608)
at builtin/index-pack.c:486
#3 0x00434f6e in resolve_delta (delta_obj=0xd82608, base=0xd891e0,
result=0x12b01f0) at builtin/index-pack.c:687
#4 0x0043528a in find_unresolved_deltas_1 (base=0xd891e0, prev_base=0x0)
at builtin/index-pack.c:743
#5 0x004352ee in find_unresolved_deltas (base=0xd891e0)
at builtin/index-pack.c:759
#6 0x004353cd in second_pass (obj=0xd81bc0) at builtin/index-pack.c:798
#7 0x004354c4 in threaded_second_pass (arg=0xd811e8)
at builtin/index-pack.c:821
#8 0x00505200 in win32_start_routine (arg=0xd811e8)
at compat/win32/pthread.c:20
#9 ...
I don't see any mutual exclusion happening in this chain. Perhaps it is
not needed, provided that the pread() call in get_data_from_pack is
atomic. But our git_pread() from compat/pread.c, which we use on Windows,
is not atomic.
:-(
I don't think that it is possible to make git_pread() atomic because it
would have to be protected against all file accesses that can modify the
file position.
Is get_data_from_pack() the only function that accesses the pack data?
Then we could add some mutual exclusion there.
-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html