* t5702 failing under ASan on master
@ 2019-01-30 8:58 brian m. carlson
2019-01-30 10:07 ` Duy Nguyen
2019-02-04 0:06 ` [PATCH] fetch-pack: clear alternate shallow when complete brian m. carlson
0 siblings, 2 replies; 13+ messages in thread
From: brian m. carlson @ 2019-01-30 8:58 UTC (permalink / raw)
To: git; +Cc: Duy Nguyen, Jonathan Tan
[-- Attachment #1: Type: text/plain, Size: 21225 bytes --]
It looks like t5702 is failing on master under ASan (output below). It
bisects to the merge of my sha-256 series, but the error makes it look
like it's unrelated. I'm wondering if we just happened to have a
different memory layout with that series that's triggering this issue in
combination with one of the protocol v2 series from Jonathan Tan, and
the correct solution is something like this:
----- %< -------
diff --git a/fetch-pack.c b/fetch-pack.c
index 577faa6229..c9dda154da 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1489,6 +1489,7 @@ static void update_shallow(struct fetch_pack_args *args,
rollback_lock_file(&shallow_lock);
} else
commit_lock_file(&shallow_lock);
+ alternate_shallow_file = "";
return;
}
@@ -1512,6 +1513,7 @@ static void update_shallow(struct fetch_pack_args *args,
&alternate_shallow_file,
&extra);
commit_lock_file(&shallow_lock);
+ alternate_shallow_file = "";
}
oid_array_clear(&extra);
return;
@@ -1551,6 +1553,7 @@ static void update_shallow(struct fetch_pack_args *args,
commit_lock_file(&shallow_lock);
oid_array_clear(&extra);
oid_array_clear(&ref);
+ alternate_shallow_file = "";
return;
}
----- %< -------
This does appear to pass the testsuite, but I'm unsure if it's correct.
It's also possible I've just broken something and am too dense to
realize it, so please point out if that's the case.
Output:
expecting success:
rm -rf server client trace &&
test_create_repo server &&
test_commit -C server one &&
test_commit -C server two &&
test_commit -C server three &&
git clone --shallow-exclude two "file://$(pwd)/server" client &&
git -C server tag -a -m "an annotated tag" twotag two &&
# Triggers tag following (thus, 2 fetches in one process)
GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \
fetch --shallow-exclude one origin &&
# Ensure that protocol v2 is used
grep "fetch< version 2" trace
Initialized empty Git repository in /home/bmc/checkouts/git/t/trash directory.t5702-protocol-v2/server/.git/
[master (root-commit) 1581e3e] one
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+)
create mode 100644 one.t
[master 5680d21] two
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+)
create mode 100644 two.t
[master 94705d7] three
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+)
create mode 100644 three.t
Cloning into 'client'...
remote: Enumerating objects: 5, done.
remote: Counting objects: 100% (5/5), done.
remote: Compressing objects: 100% (2/2), done.
remote: Total 5 (delta 0), reused 0 (delta 0)
Receiving objects: 100% (5/5), done.
remote: Enumerating objects: 4, done.
remote: Counting objects: 100% (4/4), done.
remote: Compressing objects: 100% (3/3), done.
remote: Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
remote: Enumerating objects: 1, done.
remote: Counting objects: 100% (1/1), done.
remote: Total 1 (delta 0), reused 0 (delta 0)
=================================================================
==628655==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c000000a00 at pc 0x14c115e7e8cd bp 0x7ffe4f405a00 sp 0x7ffe4f4051b0
READ of size 2 at 0x60c000000a00 thread T0
#0 0x14c115e7e8cc in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a8cc)
#1 0x5572f1a4ebe0 in xstrdup /home/bmc/checkouts/git/wrapper.c:44
#2 0x5572f1773f33 in argv_array_push /home/bmc/checkouts/git/argv-array.c:26
#3 0x5572f183b380 in get_pack /home/bmc/checkouts/git/fetch-pack.c:786
#4 0x5572f183fb59 in do_fetch_pack_v2 /home/bmc/checkouts/git/fetch-pack.c:1391
#5 0x5572f183fb59 in fetch_pack /home/bmc/checkouts/git/fetch-pack.c:1620
#6 0x5572f1a1172a in fetch_refs_via_pack /home/bmc/checkouts/git/transport.c:365
#7 0x5572f1a1554d in transport_fetch_refs /home/bmc/checkouts/git/transport.c:1296
#8 0x5572f1623b84 in fetch_refs builtin/fetch.c:1016
#9 0x5572f162b056 in backfill_tags builtin/fetch.c:1221
#10 0x5572f162b056 in do_fetch builtin/fetch.c:1321
#11 0x5572f162b056 in fetch_one builtin/fetch.c:1551
#12 0x5572f162b056 in cmd_fetch builtin/fetch.c:1640
#13 0x5572f15af748 in run_builtin /home/bmc/checkouts/git/git.c:422
#14 0x5572f15af748 in handle_builtin /home/bmc/checkouts/git/git.c:648
#15 0x5572f15b2651 in run_argv /home/bmc/checkouts/git/git.c:702
#16 0x5572f15b2651 in cmd_main /home/bmc/checkouts/git/git.c:799
#17 0x5572f15ad5e3 in main /home/bmc/checkouts/git/common-main.c:45
#18 0x14c115a5e09a in __libc_start_main ../csu/libc-start.c:308
#19 0x5572f15aef89 in _start (/home/bmc/checkouts/git/git+0x169f89)
0x60c000000a00 is located 0 bytes inside of 124-byte region [0x60c000000a00,0x60c000000a7c)
freed by thread T0 here:
#0 0x14c115f2cb70 in free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe8b70)
#1 0x5572f19e56ae in strbuf_release /home/bmc/checkouts/git/strbuf.c:64
#2 0x5572f1a04e85 in deactivate_tempfile /home/bmc/checkouts/git/tempfile.c:127
#3 0x5572f1a0625a in rename_tempfile /home/bmc/checkouts/git/tempfile.c:305
#4 0x5572f187e0b2 in commit_lock_file_to /home/bmc/checkouts/git/lockfile.h:293
#5 0x5572f187e0b2 in commit_lock_file /home/bmc/checkouts/git/lockfile.c:206
#6 0x5572f183f463 in update_shallow /home/bmc/checkouts/git/fetch-pack.c:1491
#7 0x5572f183f463 in fetch_pack /home/bmc/checkouts/git/fetch-pack.c:1643
#8 0x5572f1a1172a in fetch_refs_via_pack /home/bmc/checkouts/git/transport.c:365
#9 0x5572f1a1554d in transport_fetch_refs /home/bmc/checkouts/git/transport.c:1296
#10 0x5572f1623b84 in fetch_refs builtin/fetch.c:1016
#11 0x5572f1629fbd in do_fetch builtin/fetch.c:1307
#12 0x5572f1629fbd in fetch_one builtin/fetch.c:1551
#13 0x5572f1629fbd in cmd_fetch builtin/fetch.c:1640
#14 0x5572f15af748 in run_builtin /home/bmc/checkouts/git/git.c:422
#15 0x5572f15af748 in handle_builtin /home/bmc/checkouts/git/git.c:648
#16 0x5572f15b2651 in run_argv /home/bmc/checkouts/git/git.c:702
#17 0x5572f15b2651 in cmd_main /home/bmc/checkouts/git/git.c:799
#18 0x5572f15ad5e3 in main /home/bmc/checkouts/git/common-main.c:45
#19 0x14c115a5e09a in __libc_start_main ../csu/libc-start.c:308
previously allocated by thread T0 here:
#0 0x14c115f2d2e0 in realloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe92e0)
#1 0x5572f1a4ed01 in xrealloc /home/bmc/checkouts/git/wrapper.c:138
#2 0x5572f19e5b60 in strbuf_grow /home/bmc/checkouts/git/strbuf.c:98
#3 0x5572f19ea7e3 in strbuf_addch /home/bmc/checkouts/git/strbuf.h:231
#4 0x5572f19ea7e3 in strbuf_add_absolute_path /home/bmc/checkouts/git/strbuf.c:780
#5 0x5572f1a05da5 in create_tempfile /home/bmc/checkouts/git/tempfile.c:137
#6 0x5572f187d12c in lock_file /home/bmc/checkouts/git/lockfile.c:82
#7 0x5572f187db28 in lock_file_timeout /home/bmc/checkouts/git/lockfile.c:110
#8 0x5572f187db28 in hold_lock_file_for_update_timeout /home/bmc/checkouts/git/lockfile.c:175
#9 0x5572f19de0f1 in hold_lock_file_for_update /home/bmc/checkouts/git/lockfile.h:175
#10 0x5572f19de0f1 in setup_alternate_shallow /home/bmc/checkouts/git/shallow.c:350
#11 0x5572f1841499 in receive_shallow_info /home/bmc/checkouts/git/fetch-pack.c:1272
#12 0x5572f1841499 in do_fetch_pack_v2 /home/bmc/checkouts/git/fetch-pack.c:1384
#13 0x5572f1841499 in fetch_pack /home/bmc/checkouts/git/fetch-pack.c:1620
#14 0x5572f1a1172a in fetch_refs_via_pack /home/bmc/checkouts/git/transport.c:365
#15 0x5572f1a1554d in transport_fetch_refs /home/bmc/checkouts/git/transport.c:1296
#16 0x5572f1623b84 in fetch_refs builtin/fetch.c:1016
#17 0x5572f1629fbd in do_fetch builtin/fetch.c:1307
#18 0x5572f1629fbd in fetch_one builtin/fetch.c:1551
#19 0x5572f1629fbd in cmd_fetch builtin/fetch.c:1640
#20 0x5572f15af748 in run_builtin /home/bmc/checkouts/git/git.c:422
#21 0x5572f15af748 in handle_builtin /home/bmc/checkouts/git/git.c:648
#22 0x5572f15b2651 in run_argv /home/bmc/checkouts/git/git.c:702
#23 0x5572f15b2651 in cmd_main /home/bmc/checkouts/git/git.c:799
#24 0x5572f15ad5e3 in main /home/bmc/checkouts/git/common-main.c:45
#25 0x14c115a5e09a in __libc_start_main ../csu/libc-start.c:308
SUMMARY: AddressSanitizer: heap-use-after-free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a8cc) in strdup
Shadow bytes around the buggy address:
0x0c187fff80f0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
0x0c187fff8100: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
0x0c187fff8110: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
0x0c187fff8120: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
0x0c187fff8130: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
=>0x0c187fff8140:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c187fff8150: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
0x0c187fff8160: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
0x0c187fff8170: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
0x0c187fff8180: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
0x0c187fff8190: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==628655==ABORTING
Aborted
not ok 30 - ensure that multiple fetches in same process from a shallow repo works
#
# rm -rf server client trace &&
#
# test_create_repo server &&
# test_commit -C server one &&
# test_commit -C server two &&
# test_commit -C server three &&
# git clone --shallow-exclude two "file://$(pwd)/server" client &&
#
# git -C server tag -a -m "an annotated tag" twotag two &&
#
# # Triggers tag following (thus, 2 fetches in one process)
# GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \
# fetch --shallow-exclude one origin &&
# # Ensure that protocol v2 is used
# grep "fetch< version 2" trace
#
expecting success:
rm -rf server client trace &&
test_create_repo server &&
test_commit -C server one &&
test_commit -C server two &&
test_commit -C server three &&
git clone --depth 1 "file://$(pwd)/server" client &&
test_commit -C server four &&
# Sanity check that only "three" is downloaded
git -C client log --pretty=tformat:%s master >actual &&
echo three >expected &&
test_cmp expected actual &&
GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \
fetch --deepen=1 origin &&
# Ensure that protocol v2 is used
grep "fetch< version 2" trace &&
git -C client log --pretty=tformat:%s origin/master >actual &&
cat >expected <<-\EOF &&
four
three
two
EOF
test_cmp expected actual
Initialized empty Git repository in /home/bmc/checkouts/git/t/trash directory.t5702-protocol-v2/server/.git/
[master (root-commit) 3735e1b] one
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+)
create mode 100644 one.t
[master 581c9db] two
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+)
create mode 100644 two.t
[master f6e0668] three
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+)
create mode 100644 three.t
Cloning into 'client'...
remote: Enumerating objects: 5, done.
remote: Counting objects: 100% (5/5), done.
remote: Compressing objects: 100% (2/2), done.
remote: Total 5 (delta 0), reused 0 (delta 0)
Receiving objects: 100% (5/5), done.
[master e79f779] four
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+)
create mode 100644 four.t
remote: Enumerating objects: 6, done.
remote: Counting objects: 100% (6/6), done.
remote: Compressing objects: 100% (4/4), done.
remote: Total 5 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (5/5), done.
From file:///home/bmc/checkouts/git/t/trash directory.t5702-protocol-v2/server
f6e0668..e79f779 master -> origin/master
* [new tag] four -> four
=================================================================
remote: Total 0 (delta 0), reused 0 (delta 0)
==628707==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c000000ac0 at pc 0x1519f5dd78cd bp 0x7ffe67609620 sp 0x7ffe67608dd0
READ of size 2 at 0x60c000000ac0 thread T0
#0 0x1519f5dd78cc in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a8cc)
#1 0x561ad07c7be0 in xstrdup /home/bmc/checkouts/git/wrapper.c:44
#2 0x561ad04ecf33 in argv_array_push /home/bmc/checkouts/git/argv-array.c:26
#3 0x561ad05b4380 in get_pack /home/bmc/checkouts/git/fetch-pack.c:786
#4 0x561ad05b8b59 in do_fetch_pack_v2 /home/bmc/checkouts/git/fetch-pack.c:1391
#5 0x561ad05b8b59 in fetch_pack /home/bmc/checkouts/git/fetch-pack.c:1620
#6 0x561ad078a72a in fetch_refs_via_pack /home/bmc/checkouts/git/transport.c:365
#7 0x561ad078e54d in transport_fetch_refs /home/bmc/checkouts/git/transport.c:1296
#8 0x561ad039cb84 in fetch_refs builtin/fetch.c:1016
#9 0x561ad03a4056 in backfill_tags builtin/fetch.c:1221
#10 0x561ad03a4056 in do_fetch builtin/fetch.c:1321
#11 0x561ad03a4056 in fetch_one builtin/fetch.c:1551
#12 0x561ad03a4056 in cmd_fetch builtin/fetch.c:1640
#13 0x561ad0328748 in run_builtin /home/bmc/checkouts/git/git.c:422
#14 0x561ad0328748 in handle_builtin /home/bmc/checkouts/git/git.c:648
#15 0x561ad032b651 in run_argv /home/bmc/checkouts/git/git.c:702
#16 0x561ad032b651 in cmd_main /home/bmc/checkouts/git/git.c:799
#17 0x561ad03265e3 in main /home/bmc/checkouts/git/common-main.c:45
#18 0x1519f59b709a in __libc_start_main ../csu/libc-start.c:308
#19 0x561ad0327f89 in _start (/home/bmc/checkouts/git/git+0x169f89)
0x60c000000ac0 is located 0 bytes inside of 124-byte region [0x60c000000ac0,0x60c000000b3c)
freed by thread T0 here:
#0 0x1519f5e85b70 in free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe8b70)
#1 0x561ad075e6ae in strbuf_release /home/bmc/checkouts/git/strbuf.c:64
#2 0x561ad077de85 in deactivate_tempfile /home/bmc/checkouts/git/tempfile.c:127
#3 0x561ad077f25a in rename_tempfile /home/bmc/checkouts/git/tempfile.c:305
#4 0x561ad05f70b2 in commit_lock_file_to /home/bmc/checkouts/git/lockfile.h:293
#5 0x561ad05f70b2 in commit_lock_file /home/bmc/checkouts/git/lockfile.c:206
#6 0x561ad05b8463 in update_shallow /home/bmc/checkouts/git/fetch-pack.c:1491
#7 0x561ad05b8463 in fetch_pack /home/bmc/checkouts/git/fetch-pack.c:1643
#8 0x561ad078a72a in fetch_refs_via_pack /home/bmc/checkouts/git/transport.c:365
#9 0x561ad078e54d in transport_fetch_refs /home/bmc/checkouts/git/transport.c:1296
#10 0x561ad039cb84 in fetch_refs builtin/fetch.c:1016
#11 0x561ad03a2fbd in do_fetch builtin/fetch.c:1307
#12 0x561ad03a2fbd in fetch_one builtin/fetch.c:1551
#13 0x561ad03a2fbd in cmd_fetch builtin/fetch.c:1640
#14 0x561ad0328748 in run_builtin /home/bmc/checkouts/git/git.c:422
#15 0x561ad0328748 in handle_builtin /home/bmc/checkouts/git/git.c:648
#16 0x561ad032b651 in run_argv /home/bmc/checkouts/git/git.c:702
#17 0x561ad032b651 in cmd_main /home/bmc/checkouts/git/git.c:799
#18 0x561ad03265e3 in main /home/bmc/checkouts/git/common-main.c:45
#19 0x1519f59b709a in __libc_start_main ../csu/libc-start.c:308
previously allocated by thread T0 here:
#0 0x1519f5e862e0 in realloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xe92e0)
#1 0x561ad07c7d01 in xrealloc /home/bmc/checkouts/git/wrapper.c:138
#2 0x561ad075eb60 in strbuf_grow /home/bmc/checkouts/git/strbuf.c:98
#3 0x561ad07637e3 in strbuf_addch /home/bmc/checkouts/git/strbuf.h:231
#4 0x561ad07637e3 in strbuf_add_absolute_path /home/bmc/checkouts/git/strbuf.c:780
#5 0x561ad077eda5 in create_tempfile /home/bmc/checkouts/git/tempfile.c:137
#6 0x561ad05f612c in lock_file /home/bmc/checkouts/git/lockfile.c:82
#7 0x561ad05f6b28 in lock_file_timeout /home/bmc/checkouts/git/lockfile.c:110
#8 0x561ad05f6b28 in hold_lock_file_for_update_timeout /home/bmc/checkouts/git/lockfile.c:175
#9 0x561ad07570f1 in hold_lock_file_for_update /home/bmc/checkouts/git/lockfile.h:175
#10 0x561ad07570f1 in setup_alternate_shallow /home/bmc/checkouts/git/shallow.c:350
#11 0x561ad05ba499 in receive_shallow_info /home/bmc/checkouts/git/fetch-pack.c:1272
#12 0x561ad05ba499 in do_fetch_pack_v2 /home/bmc/checkouts/git/fetch-pack.c:1384
#13 0x561ad05ba499 in fetch_pack /home/bmc/checkouts/git/fetch-pack.c:1620
#14 0x561ad078a72a in fetch_refs_via_pack /home/bmc/checkouts/git/transport.c:365
#15 0x561ad078e54d in transport_fetch_refs /home/bmc/checkouts/git/transport.c:1296
#16 0x561ad039cb84 in fetch_refs builtin/fetch.c:1016
#17 0x561ad03a2fbd in do_fetch builtin/fetch.c:1307
#18 0x561ad03a2fbd in fetch_one builtin/fetch.c:1551
#19 0x561ad03a2fbd in cmd_fetch builtin/fetch.c:1640
#20 0x561ad0328748 in run_builtin /home/bmc/checkouts/git/git.c:422
#21 0x561ad0328748 in handle_builtin /home/bmc/checkouts/git/git.c:648
#22 0x561ad032b651 in run_argv /home/bmc/checkouts/git/git.c:702
#23 0x561ad032b651 in cmd_main /home/bmc/checkouts/git/git.c:799
#24 0x561ad03265e3 in main /home/bmc/checkouts/git/common-main.c:45
#25 0x1519f59b709a in __libc_start_main ../csu/libc-start.c:308
SUMMARY: AddressSanitizer: heap-use-after-free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x3a8cc) in strdup
Shadow bytes around the buggy address:
0x0c187fff8100: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
0x0c187fff8110: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
0x0c187fff8120: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
0x0c187fff8130: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa
0x0c187fff8140: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c187fff8150: fa fa fa fa fa fa fa fa[fd]fd fd fd fd fd fd fd
0x0c187fff8160: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
0x0c187fff8170: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c187fff8180: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
0x0c187fff8190: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
0x0c187fff81a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==628707==ABORTING
Aborted
not ok 31 - deepen-relative
#
# rm -rf server client trace &&
#
# test_create_repo server &&
# test_commit -C server one &&
# test_commit -C server two &&
# test_commit -C server three &&
# git clone --depth 1 "file://$(pwd)/server" client &&
# test_commit -C server four &&
#
# # Sanity check that only "three" is downloaded
# git -C client log --pretty=tformat:%s master >actual &&
# echo three >expected &&
# test_cmp expected actual &&
#
# GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \
# fetch --deepen=1 origin &&
# # Ensure that protocol v2 is used
# grep "fetch< version 2" trace &&
#
# git -C client log --pretty=tformat:%s origin/master >actual &&
# cat >expected <<-\EOF &&
# four
# three
# two
# EOF
# test_cmp expected actual
#
--
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: t5702 failing under ASan on master
2019-01-30 8:58 t5702 failing under ASan on master brian m. carlson
@ 2019-01-30 10:07 ` Duy Nguyen
2019-01-30 11:40 ` brian m. carlson
2019-02-04 0:06 ` [PATCH] fetch-pack: clear alternate shallow when complete brian m. carlson
1 sibling, 1 reply; 13+ messages in thread
From: Duy Nguyen @ 2019-01-30 10:07 UTC (permalink / raw)
To: brian m. carlson, Git Mailing List, Duy Nguyen, Jonathan Tan
On Wed, Jan 30, 2019 at 3:59 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> It looks like t5702 is failing on master under ASan (output below). It
> bisects to the merge of my sha-256 series, but the error makes it look
> like it's unrelated. I'm wondering if we just happened to have a
> different memory layout with that series that's triggering this issue in
> combination with one of the protocol v2 series from Jonathan Tan, and
> the correct solution is something like this:
>
> ----- %< -------
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 577faa6229..c9dda154da 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1489,6 +1489,7 @@ static void update_shallow(struct fetch_pack_args *args,
> rollback_lock_file(&shallow_lock);
> } else
> commit_lock_file(&shallow_lock);
> + alternate_shallow_file = "";
> return;
> }
>
> @@ -1512,6 +1513,7 @@ static void update_shallow(struct fetch_pack_args *args,
> &alternate_shallow_file,
> &extra);
> commit_lock_file(&shallow_lock);
> + alternate_shallow_file = "";
> }
> oid_array_clear(&extra);
> return;
> @@ -1551,6 +1553,7 @@ static void update_shallow(struct fetch_pack_args *args,
> commit_lock_file(&shallow_lock);
> oid_array_clear(&extra);
> oid_array_clear(&ref);
> + alternate_shallow_file = "";
> return;
> }
>
> ----- %< -------
>
> This does appear to pass the testsuite, but I'm unsure if it's correct.
> It's also possible I've just broken something and am too dense to
> realize it, so please point out if that's the case.
If I understand ASan report correctly alternate_shallow_file memory is
already gone after the first fetch, when we update the shallow file.
But we're doing two fetches in the same process (the tag backfill
thingy), the second fetch reuses the dangling alternate_shallow_file
pointer and ASan caught it. Resetting the variable seems like the
right way to go.
But should we reset it to an empty string? We would pass
"--shallow-file=" to "git index-pack", which is treated as "no shallow
file" (i.e. complete repo). This sounds wrong because this is still a
shallow repository.
I suppose setting alternate_shallow_file to NULL would be ok. "git
index-pack" will just go back to reading $GIT_DIR/info/shallow, which
has been updated and contains correct info.
PS. No idea how ASan blames your series for this. Yeah maybe memory
layout and stuff. But it does spot a real problem.
--
Duy
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: t5702 failing under ASan on master
2019-01-30 10:07 ` Duy Nguyen
@ 2019-01-30 11:40 ` brian m. carlson
2019-01-31 18:12 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: brian m. carlson @ 2019-01-30 11:40 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Jonathan Tan
[-- Attachment #1: Type: text/plain, Size: 1417 bytes --]
On Wed, Jan 30, 2019 at 05:07:20PM +0700, Duy Nguyen wrote:
> If I understand ASan report correctly alternate_shallow_file memory is
> already gone after the first fetch, when we update the shallow file.
> But we're doing two fetches in the same process (the tag backfill
> thingy), the second fetch reuses the dangling alternate_shallow_file
> pointer and ASan caught it. Resetting the variable seems like the
> right way to go.
Ah, I think I was missing the fact that we're doing a tag backfill. That
explains a lot.
> But should we reset it to an empty string? We would pass
> "--shallow-file=" to "git index-pack", which is treated as "no shallow
> file" (i.e. complete repo). This sounds wrong because this is still a
> shallow repository.
>
> I suppose setting alternate_shallow_file to NULL would be ok. "git
> index-pack" will just go back to reading $GIT_DIR/info/shallow, which
> has been updated and contains correct info.
Yeah, that sounds like a better choice. I'll send a complete patch which
does this.
> PS. No idea how ASan blames your series for this. Yeah maybe memory
> layout and stuff. But it does spot a real problem.
I don't doubt this is a problem. We'll definitely want to fix it before
the release, since if I see it in development, somebody will likely see
it in production.
--
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: t5702 failing under ASan on master
2019-01-30 11:40 ` brian m. carlson
@ 2019-01-31 18:12 ` Junio C Hamano
0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2019-01-31 18:12 UTC (permalink / raw)
To: brian m. carlson; +Cc: Duy Nguyen, Git Mailing List, Jonathan Tan
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> On Wed, Jan 30, 2019 at 05:07:20PM +0700, Duy Nguyen wrote:
>> If I understand ASan report correctly alternate_shallow_file memory is
>> already gone after the first fetch, when we update the shallow file.
>> But we're doing two fetches in the same process (the tag backfill
>> thingy), the second fetch reuses the dangling alternate_shallow_file
>> pointer and ASan caught it. Resetting the variable seems like the
>> right way to go.
>
> Ah, I think I was missing the fact that we're doing a tag backfill. That
> explains a lot.
>
>> But should we reset it to an empty string? We would pass
>> "--shallow-file=" to "git index-pack", which is treated as "no shallow
>> file" (i.e. complete repo). This sounds wrong because this is still a
>> shallow repository.
>>
>> I suppose setting alternate_shallow_file to NULL would be ok. "git
>> index-pack" will just go back to reading $GIT_DIR/info/shallow, which
>> has been updated and contains correct info.
>
> Yeah, that sounds like a better choice. I'll send a complete patch which
> does this.
Thanks, both.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] fetch-pack: clear alternate shallow when complete
2019-01-30 8:58 t5702 failing under ASan on master brian m. carlson
2019-01-30 10:07 ` Duy Nguyen
@ 2019-02-04 0:06 ` brian m. carlson
2019-02-04 10:34 ` Duy Nguyen
2019-02-06 23:59 ` [PATCH v2] " brian m. carlson
1 sibling, 2 replies; 13+ messages in thread
From: brian m. carlson @ 2019-02-04 0:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Duy Nguyen
When we write an alternate shallow file in update_shallow, we write it
into the lock file. The string stored in alternate_shallow_file is
copied from the lock file path, but it is freed the moment that the lock
file is closed, since we call strbuf_release to free that path.
This used to work, since we did not invoke git index-pack more than
once, but now that we do, we reuse the freed memory. Ensure we reset the
value to NULL to avoid using freed memory. git index-pack will read the
repository's shallow file, which will have been updated with the correct
information.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
fetch-pack.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fetch-pack.c b/fetch-pack.c
index 577faa6229..2d76287674 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1489,6 +1489,7 @@ static void update_shallow(struct fetch_pack_args *args,
rollback_lock_file(&shallow_lock);
} else
commit_lock_file(&shallow_lock);
+ alternate_shallow_file = NULL;
return;
}
@@ -1512,6 +1513,7 @@ static void update_shallow(struct fetch_pack_args *args,
&alternate_shallow_file,
&extra);
commit_lock_file(&shallow_lock);
+ alternate_shallow_file = NULL;
}
oid_array_clear(&extra);
return;
@@ -1551,6 +1553,7 @@ static void update_shallow(struct fetch_pack_args *args,
commit_lock_file(&shallow_lock);
oid_array_clear(&extra);
oid_array_clear(&ref);
+ alternate_shallow_file = NULL;
return;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] fetch-pack: clear alternate shallow when complete
2019-02-04 0:06 ` [PATCH] fetch-pack: clear alternate shallow when complete brian m. carlson
@ 2019-02-04 10:34 ` Duy Nguyen
2019-02-05 16:26 ` Jonathan Tan
2019-02-06 23:59 ` [PATCH v2] " brian m. carlson
1 sibling, 1 reply; 13+ messages in thread
From: Duy Nguyen @ 2019-02-04 10:34 UTC (permalink / raw)
To: brian m. carlson, Jonathan Tan, Jonathan Nieder, Stefan Beller
Cc: Junio C Hamano, Git Mailing List
On Mon, Feb 4, 2019 at 7:06 AM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> When we write an alternate shallow file in update_shallow, we write it
> into the lock file. The string stored in alternate_shallow_file is
> copied from the lock file path, but it is freed the moment that the lock
> file is closed, since we call strbuf_release to free that path.
>
> This used to work, since we did not invoke git index-pack more than
> once, but now that we do, we reuse the freed memory. Ensure we reset the
> value to NULL to avoid using freed memory. git index-pack will read the
> repository's shallow file, which will have been updated with the correct
> information.
It may be worth mentioning bd0b42aed3 (fetch-pack: do not take shallow
lock unnecessarily - 2019-01-10). I believe this is the same problem
and a full solution was suggested but not implemented in that commit.
The problem with dangling alternate_shallow_file is also from that
commit. When line_received is false at the end of
receive_shallow_info(), we should clear alternate_shallow_file. I'm
still debating myself whether we should clear alternate_shallow_file
in receive_shallow_info() in addition to your changes (which is good
hygiene anyway) to keep the setup steps of do_fetch_pack() and
do_fetch_pack_v2() aligned.
pack protocol v1 does this clearing alternate_shallow_file (near the
end of do_fetch_pack()) so it will not have the same dangling pointer
problem even if we do two fetches in the same process. But from the
Tan's commit description, I thought v1 probably faces the same "lock
twice not supported". But luckily it's never triggered in practice.
When backfill_tags() is called, if I read it correctly, we either
disable shallow (TRANS_OPT_DEPTH set to zero) or we start a second
process. In either case, the lock will not be acquired twice by the
same process.
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> fetch-pack.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 577faa6229..2d76287674 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1489,6 +1489,7 @@ static void update_shallow(struct fetch_pack_args *args,
> rollback_lock_file(&shallow_lock);
> } else
> commit_lock_file(&shallow_lock);
> + alternate_shallow_file = NULL;
> return;
> }
>
> @@ -1512,6 +1513,7 @@ static void update_shallow(struct fetch_pack_args *args,
> &alternate_shallow_file,
> &extra);
> commit_lock_file(&shallow_lock);
> + alternate_shallow_file = NULL;
> }
> oid_array_clear(&extra);
> return;
> @@ -1551,6 +1553,7 @@ static void update_shallow(struct fetch_pack_args *args,
> commit_lock_file(&shallow_lock);
> oid_array_clear(&extra);
> oid_array_clear(&ref);
> + alternate_shallow_file = NULL;
> return;
> }
>
--
Duy
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fetch-pack: clear alternate shallow when complete
2019-02-04 10:34 ` Duy Nguyen
@ 2019-02-05 16:26 ` Jonathan Tan
2019-02-06 23:26 ` brian m. carlson
0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Tan @ 2019-02-05 16:26 UTC (permalink / raw)
To: pclouds; +Cc: sandals, jonathantanmy, jrnieder, sbeller, gitster, git
> On Mon, Feb 4, 2019 at 7:06 AM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> >
> > When we write an alternate shallow file in update_shallow, we write it
> > into the lock file. The string stored in alternate_shallow_file is
> > copied from the lock file path, but it is freed the moment that the lock
> > file is closed, since we call strbuf_release to free that path.
> >
> > This used to work, since we did not invoke git index-pack more than
> > once, but now that we do, we reuse the freed memory. Ensure we reset the
> > value to NULL to avoid using freed memory. git index-pack will read the
> > repository's shallow file, which will have been updated with the correct
> > information.
>
> It may be worth mentioning bd0b42aed3 (fetch-pack: do not take shallow
> lock unnecessarily - 2019-01-10). I believe this is the same problem
> and a full solution was suggested but not implemented in that commit.
For reference, the full solution is [1], linked from that commit's email
[2]. (Looking back, I probably should have included all the information
below the "---" in the commit message proper.) The full solution is more
related to shallow locks, though, not alternate_shallow_file.
[1] https://public-inbox.org/git/20181218010811.143608-1-jonathantanmy@google.com/
[2] https://public-inbox.org/git/20190110193645.34080-1-jonathantanmy@google.com/
> The problem with dangling alternate_shallow_file is also from that
> commit.
You're right - thanks for noticing this.
> When line_received is false at the end of
> receive_shallow_info(), we should clear alternate_shallow_file. I'm
> still debating myself whether we should clear alternate_shallow_file
> in receive_shallow_info() in addition to your changes (which is good
> hygiene anyway) to keep the setup steps of do_fetch_pack() and
> do_fetch_pack_v2() aligned.
Clearing alternate_shallow_file when line_received is false at the end
of receive_shallow_info() sounds like a good idea to me.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fetch-pack: clear alternate shallow when complete
2019-02-05 16:26 ` Jonathan Tan
@ 2019-02-06 23:26 ` brian m. carlson
0 siblings, 0 replies; 13+ messages in thread
From: brian m. carlson @ 2019-02-06 23:26 UTC (permalink / raw)
To: Jonathan Tan; +Cc: pclouds, jrnieder, sbeller, gitster, git
[-- Attachment #1: Type: text/plain, Size: 1691 bytes --]
On Tue, Feb 05, 2019 at 08:26:36AM -0800, Jonathan Tan wrote:
> > It may be worth mentioning bd0b42aed3 (fetch-pack: do not take shallow
> > lock unnecessarily - 2019-01-10). I believe this is the same problem
> > and a full solution was suggested but not implemented in that commit.
>
> For reference, the full solution is [1], linked from that commit's email
> [2]. (Looking back, I probably should have included all the information
> below the "---" in the commit message proper.) The full solution is more
> related to shallow locks, though, not alternate_shallow_file.
>
> [1] https://public-inbox.org/git/20181218010811.143608-1-jonathantanmy@google.com/
> [2] https://public-inbox.org/git/20190110193645.34080-1-jonathantanmy@google.com/
>
> > The problem with dangling alternate_shallow_file is also from that
> > commit.
>
> You're right - thanks for noticing this.
>
> > When line_received is false at the end of
> > receive_shallow_info(), we should clear alternate_shallow_file. I'm
> > still debating myself whether we should clear alternate_shallow_file
> > in receive_shallow_info() in addition to your changes (which is good
> > hygiene anyway) to keep the setup steps of do_fetch_pack() and
> > do_fetch_pack_v2() aligned.
>
> Clearing alternate_shallow_file when line_received is false at the end
> of receive_shallow_info() sounds like a good idea to me.
I'll reroll with that change and a commit message update. I think it's
important that we get this issue fixed for the release, and then we can
choose to implement a more comprehensive solution later.
--
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] fetch-pack: clear alternate shallow when complete
2019-02-04 0:06 ` [PATCH] fetch-pack: clear alternate shallow when complete brian m. carlson
2019-02-04 10:34 ` Duy Nguyen
@ 2019-02-06 23:59 ` brian m. carlson
2019-02-07 2:32 ` Duy Nguyen
1 sibling, 1 reply; 13+ messages in thread
From: brian m. carlson @ 2019-02-06 23:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Duy Nguyen, Jonathan Tan
When we write an alternate shallow file in update_shallow, we write it
into the lock file. The string stored in alternate_shallow_file is
copied from the lock file path, but it is freed the moment that the lock
file is closed, since we call strbuf_release to free that path.
This used to work, since we did not invoke git index-pack more than
once. However, we now do, and starting with bd0b42aed3 (fetch-pack: do
not take shallow lock unnecessarily - 2019-01-10), we no longer reset
this value unconditionally; consequently, we reuse the freed memory.
Ensure we reset the value to NULL to avoid using freed memory. git
index-pack will read the repository's shallow file, which will have been
updated with the correct information.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
fetch-pack.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fetch-pack.c b/fetch-pack.c
index 577faa6229..a92621a388 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1272,6 +1272,8 @@ static void receive_shallow_info(struct fetch_pack_args *args,
setup_alternate_shallow(&shallow_lock, &alternate_shallow_file,
NULL);
args->deepen = 1;
+ } else {
+ alternate_shallow_file = NULL;
}
}
@@ -1489,6 +1491,7 @@ static void update_shallow(struct fetch_pack_args *args,
rollback_lock_file(&shallow_lock);
} else
commit_lock_file(&shallow_lock);
+ alternate_shallow_file = NULL;
return;
}
@@ -1512,6 +1515,7 @@ static void update_shallow(struct fetch_pack_args *args,
&alternate_shallow_file,
&extra);
commit_lock_file(&shallow_lock);
+ alternate_shallow_file = NULL;
}
oid_array_clear(&extra);
return;
@@ -1551,6 +1555,7 @@ static void update_shallow(struct fetch_pack_args *args,
commit_lock_file(&shallow_lock);
oid_array_clear(&extra);
oid_array_clear(&ref);
+ alternate_shallow_file = NULL;
return;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] fetch-pack: clear alternate shallow when complete
2019-02-06 23:59 ` [PATCH v2] " brian m. carlson
@ 2019-02-07 2:32 ` Duy Nguyen
2019-02-07 2:47 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: Duy Nguyen @ 2019-02-07 2:32 UTC (permalink / raw)
To: brian m. carlson; +Cc: Junio C Hamano, Git Mailing List, Jonathan Tan
On Thu, Feb 7, 2019 at 7:00 AM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> When we write an alternate shallow file in update_shallow, we write it
> into the lock file. The string stored in alternate_shallow_file is
> copied from the lock file path, but it is freed the moment that the lock
> file is closed, since we call strbuf_release to free that path.
>
> This used to work, since we did not invoke git index-pack more than
> once. However, we now do, and starting with bd0b42aed3 (fetch-pack: do
> not take shallow lock unnecessarily - 2019-01-10), we no longer reset
> this value unconditionally; consequently, we reuse the freed memory.
> Ensure we reset the value to NULL to avoid using freed memory. git
> index-pack will read the repository's shallow file, which will have been
> updated with the correct information.
The patch looks good to me.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> fetch-pack.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 577faa6229..a92621a388 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1272,6 +1272,8 @@ static void receive_shallow_info(struct fetch_pack_args *args,
> setup_alternate_shallow(&shallow_lock, &alternate_shallow_file,
> NULL);
> args->deepen = 1;
> + } else {
> + alternate_shallow_file = NULL;
> }
> }
>
> @@ -1489,6 +1491,7 @@ static void update_shallow(struct fetch_pack_args *args,
> rollback_lock_file(&shallow_lock);
> } else
> commit_lock_file(&shallow_lock);
> + alternate_shallow_file = NULL;
> return;
> }
>
> @@ -1512,6 +1515,7 @@ static void update_shallow(struct fetch_pack_args *args,
> &alternate_shallow_file,
> &extra);
> commit_lock_file(&shallow_lock);
> + alternate_shallow_file = NULL;
> }
> oid_array_clear(&extra);
> return;
> @@ -1551,6 +1555,7 @@ static void update_shallow(struct fetch_pack_args *args,
> commit_lock_file(&shallow_lock);
> oid_array_clear(&extra);
> oid_array_clear(&ref);
> + alternate_shallow_file = NULL;
> return;
> }
>
--
Duy
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] fetch-pack: clear alternate shallow when complete
2019-02-07 2:32 ` Duy Nguyen
@ 2019-02-07 2:47 ` Junio C Hamano
2019-02-07 2:53 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2019-02-07 2:47 UTC (permalink / raw)
To: Duy Nguyen; +Cc: brian m. carlson, Git Mailing List, Jonathan Tan
Duy Nguyen <pclouds@gmail.com> writes:
> On Thu, Feb 7, 2019 at 7:00 AM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
>>
>> When we write an alternate shallow file in update_shallow, we write it
>> into the lock file. The string stored in alternate_shallow_file is
>> copied from the lock file path, but it is freed the moment that the lock
>> file is closed, since we call strbuf_release to free that path.
>>
>> This used to work, since we did not invoke git index-pack more than
>> once. However, we now do, and starting with bd0b42aed3 (fetch-pack: do
>> not take shallow lock unnecessarily - 2019-01-10), we no longer reset
>> this value unconditionally; consequently, we reuse the freed memory.
>> Ensure we reset the value to NULL to avoid using freed memory. git
>> index-pack will read the repository's shallow file, which will have been
>> updated with the correct information.
>
> The patch looks good to me.
Thanks, all.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] fetch-pack: clear alternate shallow when complete
2019-02-07 2:47 ` Junio C Hamano
@ 2019-02-07 2:53 ` Junio C Hamano
2019-02-07 4:07 ` brian m. carlson
0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2019-02-07 2:53 UTC (permalink / raw)
To: Duy Nguyen; +Cc: brian m. carlson, Git Mailing List, Jonathan Tan
Junio C Hamano <gitster@pobox.com> writes:
>> The patch looks good to me.
>
> Thanks, all.
Oops, spoke a bit too fast. As the previous one is already in
'next', let's do this instead.
-- >8 --
From: "brian m. carlson" <sandals@crustytoothpaste.net>
Date: Wed, 6 Feb 2019 23:59:37 +0000
Subject: [PATCH] fetch-pack: clear alternate shallow in one more place
The previous one did not clear the variable in one codepath,
but we should aim to be complete.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
[jc: made a reroll into incremental, as the previous one already is
in the next branch]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
fetch-pack.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fetch-pack.c b/fetch-pack.c
index 2d76287674..a92621a388 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1272,6 +1272,8 @@ static void receive_shallow_info(struct fetch_pack_args *args,
setup_alternate_shallow(&shallow_lock, &alternate_shallow_file,
NULL);
args->deepen = 1;
+ } else {
+ alternate_shallow_file = NULL;
}
}
--
2.20.1-519-g8feddda32c
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] fetch-pack: clear alternate shallow when complete
2019-02-07 2:53 ` Junio C Hamano
@ 2019-02-07 4:07 ` brian m. carlson
0 siblings, 0 replies; 13+ messages in thread
From: brian m. carlson @ 2019-02-07 4:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Git Mailing List, Jonathan Tan
[-- Attachment #1: Type: text/plain, Size: 424 bytes --]
On Wed, Feb 06, 2019 at 06:53:50PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> >> The patch looks good to me.
> >
> > Thanks, all.
>
> Oops, spoke a bit too fast. As the previous one is already in
> 'next', let's do this instead.
That's fine with me. Sorry about the delay getting the reroll in.
--
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-02-07 4:07 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-30 8:58 t5702 failing under ASan on master brian m. carlson
2019-01-30 10:07 ` Duy Nguyen
2019-01-30 11:40 ` brian m. carlson
2019-01-31 18:12 ` Junio C Hamano
2019-02-04 0:06 ` [PATCH] fetch-pack: clear alternate shallow when complete brian m. carlson
2019-02-04 10:34 ` Duy Nguyen
2019-02-05 16:26 ` Jonathan Tan
2019-02-06 23:26 ` brian m. carlson
2019-02-06 23:59 ` [PATCH v2] " brian m. carlson
2019-02-07 2:32 ` Duy Nguyen
2019-02-07 2:47 ` Junio C Hamano
2019-02-07 2:53 ` Junio C Hamano
2019-02-07 4:07 ` brian m. carlson
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).