From ad01ac16ae302715559f8c77e8a91aad4c7d9bc4 Mon Sep 17 00:00:00 2001 From: John Bowman Date: Sat, 31 Oct 2026 21:43:35 -0700 Subject: [PATCH] hlink: re-root a hard-link group whose root is in a released file list Address the review comment on commit 350425a8 (RsyncProject/rsync#1144): "This will still abort when the hard-link group spans incremental file-list chunks. Re-root the group without retaining released file-list indices." When a group spans chunks, a member's F_HL_PREV can point into a file-list that is released before the member's finish_hard_link() walks the completion chain, and the fatal flist_for_ndx() call aborts the transfer. Two changes in finish_hard_link(): 1. The completion-chain lookup now uses the graceful (NULL) variant of flist_for_ndx() and stops the walk when an index is no longer in a live file-list. The chain only moves to earlier ndx values, so once one is released, everything further back is already finished and there is nothing left to re-link. No live file retains a released index: the finishing file's F_HL_PREV is reset on entry, and each walked member's F_HL_PREV is pointed at the finishing file. 2. A duplicate finish (the group already recorded as finished at some path) now re-roots the group at the newly finished file instead of keeping the recorded path. A duplicate finish only happens when the recorded root could not be hard-linked (e.g. EMLINK, EXDEV) and that member was written as a regular file, so that member's inode is the only one the group can still link to; later members then link to this live file instead of the stale root. The abort is reproducible locally with a small chunk-spanning fixture whose first file-list is under MIN_FILECNT_LOOKAHEAD/2, both on the series base 8718cd28 and with only commit 350425a8 applied: the hard-link lookahead at the top of generate_files() then matches the extra file-list before the group leader is itemized, so the members' F_HL_PREV pointers into the leader's file-list survive the match, and the fatal lookup aborts deterministically: "File-list index 3 not in 11 - 13 (finish_hard_link)" with "protocol incompatibility" (code 2). With this commit the same run completes with rc 23 and the group re-rooted on a fresh inode; a simulated WAN delay for the last member gives the identical abort/fix behavior (fixture and full evidence in the description of RsyncProject/rsync#1144). Also verified: a 1200-file chunk-spanning EMLINK fixture with the destination inode pinned at the ext4 65000-link cap completes in 0.2 s with rc 23 and the group re-rooted on a fresh inode; the single-flist EMLINK fixture takes 0.15 s, rc 23, all 101 members on one fresh inode; the hard-link regression tree is all rc 0 with no spurious output; the upstream hardlink/link-dest/keep-dirlinks suite (9 tests) passes. Co-authored-by: LIM Coding Assistant <298007100+statefullm@users.noreply.github.com> --- hlink.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/hlink.c b/hlink.c index 894f01a1..73f3bc83 100644 --- a/hlink.c +++ b/hlink.c @@ -528,7 +528,15 @@ void finish_hard_link(struct file_struct *file, const char *fname, int fin_ndx, while ((ndx = prev_ndx) >= 0) { int val; - flist = flist_for_ndx(ndx, "finish_hard_link"); + flist = flist_for_ndx(ndx, NULL); + if (!flist) { + /* The group can span file-lists, and the list holding an + * earlier member may have been released by the time this + * finish runs. The chain only moves to earlier ndx + * values, so everything from here on is already finished + * and there is nothing left to re-link. */ + break; + } file = flist->files[ndx - flist->ndx_start]; file->flags = (file->flags & ~FLAG_HLINK_FIRST) | FLAG_HLINK_DONE; prev_ndx = F_HL_PREV(file); @@ -559,16 +567,17 @@ void finish_hard_link(struct file_struct *file, const char *fname, int fin_ndx, exit_cleanup(RERR_MESSAGEIO); } if (CVAL(node->data, 0) != 0) { - /* A finished leader was already recorded for this group. - * This can happen when an earlier member of the group failed - * to hard-link (e.g. EMLINK or EXDEV) and was written as a - * regular file, then finished. Keep the original leader's - * path and tolerate the duplicate finish instead of aborting - * the whole transfer. (Upstream: RsyncProject/rsync#741.) */ + /* An earlier member of this group already finished and + * recorded its path as the group's root. That member + * could not be hard-linked to the original root (e.g. + * EMLINK or EXDEV), so it was written as a regular + * file: it, not the stale root, is the only inode this + * group can still link to. Re-root the group here so + * that later members link to this live file. + * (Upstream: RsyncProject/rsync#741.) */ rprintf(FINFO, - "hlink: group %d already finished at path=%s (%s); ignoring duplicate finish\n", - gnum, (char*)node->data, f_name(file, prev_name)); - return; + "hlink: group %d already finished at %s; re-rooting at %s (%s)\n", + gnum, (char*)node->data, our_name, f_name(file, prev_name)); } free(node->data); node->data = strdup(our_name);