From patchwork Mon Jan 6 09:24:25 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13927103 Received: from fout-a4-smtp.messagingengine.com (fout-a4-smtp.messagingengine.com [103.168.172.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F3ECB1D935C for ; Mon, 6 Jan 2025 09:24:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736155476; cv=none; b=VHrjWwlRxgQfzk/CJrSJbvcmhDaWUHZKFXik88rk8+ChtJ68aCxe8WWwIm6qa4PlWapgIzLqgGGLYvdYHbqwdQmaQo/CTWDP8yJ4fC4s+Sit/hDcaBSRFHsYjwn4u0oKOFWK5y/9V1963qhiQuwrJpeBMOLP1giM/WdsriHQClM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736155476; c=relaxed/simple; bh=H/CYUTwWKAzEZbr1ZqwqzdoZVLOiEQOw298hSCJiJEw=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=otRCXOCXT4PkrIDBHdLEYTF58EvOnva1Gte5EN3kWiQnRoQq3XVwk3tzdy9GfQZWQvXojnkl4xedMnRdudJaFimFlC2biiw7MNOMuGL54WBfH+Ufhx1BKUCar3161hHBWbx6oFlcbsiScqU0a2ErM0DUvVrQzuKIIjqcmxfe4jw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=SBcRAirb; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=Cvq+banE; arc=none smtp.client-ip=103.168.172.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="SBcRAirb"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="Cvq+banE" Received: from phl-compute-04.internal (phl-compute-04.phl.internal [10.202.2.44]) by mailfout.phl.internal (Postfix) with ESMTP id E252913809DA; Mon, 6 Jan 2025 04:24:32 -0500 (EST) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-04.internal (MEProxy); Mon, 06 Jan 2025 04:24:32 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1736155472; x=1736241872; bh=9au2lMPEJEFkbsD2KX3eOyhpcU6NW1a9BtwrV87kzvM=; b= SBcRAirb7BdokkmTab3H6wzwLgd5rCfAx+jOos13tVQu2mxd4uIaYFc7kAihnuzW 2FpdSRwkuW1OkouhLZrL1+gCHgCkZS5lSxDRRcO4rnuU7xporhbUxkdNPwNzoRQO jvW8MTjdotg8NSfXNZl6bh7dC9FChG1rlfWfcpauwFjM734Fw6gGb0A8ZA1j0k4L c3l/oyfF+ElD5Im6UA/rBx8SJtbxHRjK6IcpvosTD3PGy49yjaw/5+Uh4JknzSbW HJVjszhlo5o53PxGYOkv1ApwIhDGex45boRfAX/srnEW6p1+WQ5pAR9ufdyExvWj +p0QvmjpsOKxODilQ5gU+A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1736155472; x= 1736241872; bh=9au2lMPEJEFkbsD2KX3eOyhpcU6NW1a9BtwrV87kzvM=; b=C vq+banEDq9pgkT1kLb92k6FFYfsV6fRsz4v73P2L7tuHMG01tcPyety8P0heQmcW QJK6Zv8TTBu7TkWs+e8zVkA0gxfyv3EBb6fRCSxZxHmu5puUA0fgtPnjAn0wExA/ TQd5enXcKLpZ7+FucAJqehRxNTSm9NNotex2fvsv1W6G9h175iAotTlfO2Hxy2ab vJMF6YpVATSmG6D2DVNjBUxiejo7Y+afRK4HZcE23EegjChcSE86mDJe8DVxXdR2 75yl+rQyxypueq/h046dJhd4BYkM28IvAvw6dEjXOtGwfbGMx58flxEThB0umhs+ 8EPxkW42W+jmpuZ/6OTfA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefuddrudegtddgtdefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucenucfjughrpefhfffugg gtgffkfhgjvfevofesthejredtredtjeenucfhrhhomheprfgrthhrihgtkhcuufhtvghi nhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvghrnhepffeuieduje dvkeehuedvkeefffeivdeuleetkeduheejteekgedvudfgtdfgieelnecuvehluhhsthgv rhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimhdpnh gspghrtghpthhtohepfedpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtohepghhithhs thgvrhesphhosghogidrtghomhdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvg hlrdhorhhgpdhrtghpthhtohepphgvfhhfsehpvghffhdrnhgvth X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 6 Jan 2025 04:24:31 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 7db1964f (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 6 Jan 2025 09:24:28 +0000 (UTC) From: Patrick Steinhardt Date: Mon, 06 Jan 2025 10:24:25 +0100 Subject: [PATCH v2 1/3] object-file: rename variables in `check_collision()` Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250106-b4-pks-object-file-racy-collision-check-v2-1-8b3984ecbb18@pks.im> References: <20250106-b4-pks-object-file-racy-collision-check-v2-0-8b3984ecbb18@pks.im> In-Reply-To: <20250106-b4-pks-object-file-racy-collision-check-v2-0-8b3984ecbb18@pks.im> To: git@vger.kernel.org Cc: Jeff King , Junio C Hamano X-Mailer: b4 0.14.2 Rename variables used in `check_collision()` to clearly identify which file is the source and which is the destination. This will make the next step easier to reason about when we start to treat those files different from one another. Signed-off-by: Patrick Steinhardt --- object-file.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/object-file.c b/object-file.c index f84dcd2f2a7b88716ab47bc00ee7a605a82e8d21..e1989236ca87e565dea4d003f57882f257889ecf 100644 --- a/object-file.c +++ b/object-file.c @@ -1970,56 +1970,56 @@ static void write_object_file_prepare_literally(const struct git_hash_algo *algo hash_object_body(algo, &c, buf, len, oid, hdr, hdrlen); } -static int check_collision(const char *filename_a, const char *filename_b) +static int check_collision(const char *source, const char *dest) { - char buf_a[4096], buf_b[4096]; - int fd_a = -1, fd_b = -1; + char buf_source[4096], buf_dest[4096]; + int fd_source = -1, fd_dest = -1; int ret = 0; - fd_a = open(filename_a, O_RDONLY); - if (fd_a < 0) { + fd_source = open(source, O_RDONLY); + if (fd_source < 0) { if (errno != ENOENT) - ret = error_errno(_("unable to open %s"), filename_a); + ret = error_errno(_("unable to open %s"), source); goto out; } - fd_b = open(filename_b, O_RDONLY); - if (fd_b < 0) { + fd_dest = open(dest, O_RDONLY); + if (fd_dest < 0) { if (errno != ENOENT) - ret = error_errno(_("unable to open %s"), filename_b); + ret = error_errno(_("unable to open %s"), dest); goto out; } while (1) { ssize_t sz_a, sz_b; - sz_a = read_in_full(fd_a, buf_a, sizeof(buf_a)); + sz_a = read_in_full(fd_source, buf_source, sizeof(buf_source)); if (sz_a < 0) { - ret = error_errno(_("unable to read %s"), filename_a); + ret = error_errno(_("unable to read %s"), source); goto out; } - sz_b = read_in_full(fd_b, buf_b, sizeof(buf_b)); + sz_b = read_in_full(fd_dest, buf_dest, sizeof(buf_dest)); if (sz_b < 0) { - ret = error_errno(_("unable to read %s"), filename_b); + ret = error_errno(_("unable to read %s"), dest); goto out; } - if (sz_a != sz_b || memcmp(buf_a, buf_b, sz_a)) { + if (sz_a != sz_b || memcmp(buf_source, buf_dest, sz_a)) { ret = error(_("files '%s' and '%s' differ in contents"), - filename_a, filename_b); + source, dest); goto out; } - if (sz_a < sizeof(buf_a)) + if (sz_a < sizeof(buf_source)) break; } out: - if (fd_a > -1) - close(fd_a); - if (fd_b > -1) - close(fd_b); + if (fd_source > -1) + close(fd_source); + if (fd_dest > -1) + close(fd_dest); return ret; } From patchwork Mon Jan 6 09:24:26 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13927104 Received: from fhigh-a8-smtp.messagingengine.com (fhigh-a8-smtp.messagingengine.com [103.168.172.159]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 70A2B2AD16 for ; Mon, 6 Jan 2025 09:24:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.159 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736155477; cv=none; b=qdF+bvwYXFApXbD3xFB7Qa7HHKbEpJk9QqbMciOxzI5VXBq5NZjLgNs+hPeR+W71EGSuwnB6cGnU4OattBCR/Mc6PXIWa1sKGdNULIz+ZlLS/izPcFntUUzysXCBNdCJ3SA5XKSua8/sxy0oupr51JCKkykEwdpH5S4OgSHR1w0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736155477; c=relaxed/simple; bh=+jAbNhJrIvU45E46ceAq16AWwlniSeNs/Xm1qKt0p9Q=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=A0L7CGmp5KQPkDQNbAvdAdBRsHxe6yc0IxL9fVDl5aMVMaD3vTSlNGBOTjbUcCTWwyKtujCKqauUauCYHzTE7aBk7U83C8hk7E8huTNHY8wp7yMaK4sOm3gaqXZmibvpPey7R/WPGXwKVaBKA7Kt+xro6z3PNCrR+1QAP6CHzLM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=X1Hp7bjT; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=ZKqrE1V9; arc=none smtp.client-ip=103.168.172.159 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="X1Hp7bjT"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="ZKqrE1V9" Received: from phl-compute-03.internal (phl-compute-03.phl.internal [10.202.2.43]) by mailfhigh.phl.internal (Postfix) with ESMTP id 1FB4B1140624; Mon, 6 Jan 2025 04:24:33 -0500 (EST) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-03.internal (MEProxy); Mon, 06 Jan 2025 04:24:33 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1736155473; x=1736241873; bh=K0jlqDJXd8mNX8WHGUm6EzWKGWArKGngqLzSxwWr+ro=; b= X1Hp7bjTsZbIrcdpTXZcvb/Ko71OHBPWq48WXRBsbPAtQJO0WXNadwZ1LoliLEii 4FK4YdL47p1Aa/+/TyN0HY8JFO/bSyYdjuFAlYL/xqUi+ka3e6hD768u0B8FuCyX q7HcxkpA66dTqT4yZef+jciesMg0xARSN7u+sbt9vDVduViD/gMTlJWKgs/NKszw V1993+FlZtXlYp+uSBnMJZiP1ZALGT6XpKNxGzgJQn1fF3z3fkH/oU3jxTk5SvTw ZcQPFLckrPuexbIAFidk3WRcOj3K4yjL4dW424OJ0SdEb1obtV3G/kPHYvEkgGsS MJnz3pmzxoBEkXUADmKr4A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1736155473; x= 1736241873; bh=K0jlqDJXd8mNX8WHGUm6EzWKGWArKGngqLzSxwWr+ro=; b=Z KqrE1V9HUtLsgJ744GNJU3jGtM1WgB+OvADvNGY/E7vD/KB0yrI+NwbHEWCjCcoW BuQQ4/Fytg1aaeklLMy9kvMjzXMqfU7OucuXhAqxmwPNgWwXslUrovpgwp6RjJG7 vgQgDcYfUdQn9eNomx/hCpkV/xnbrUMgAQU35pKV/J9/SR0JVztFcvTF/pKGeeGM LBDttXchFaBSAGcPlTLgmBXXQhH54NtXgs+xG+nu9WsUaimLkATMsOva/vvebHmn VzP+WQGA8CeWSylTE8BRIZIfh4QSU1K9v2AmMzCUJ9yZZ+S/U2f/3aL6STRDv7iR MfYe+FwGceR74XmwKzDAQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefuddrudegtddgtddvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucenucfjughrpefhfffugg gtgffkfhgjvfevofesthejredtredtjeenucfhrhhomheprfgrthhrihgtkhcuufhtvghi nhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvghrnhepffeuieduje dvkeehuedvkeefffeivdeuleetkeduheejteekgedvudfgtdfgieelnecuvehluhhsthgv rhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimhdpnh gspghrtghpthhtohepfedpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtohepghhithhs thgvrhesphhosghogidrtghomhdprhgtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvg hlrdhorhhgpdhrtghpthhtohepphgvfhhfsehpvghffhdrnhgvth X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 6 Jan 2025 04:24:32 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 3f7212bc (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 6 Jan 2025 09:24:29 +0000 (UTC) From: Patrick Steinhardt Date: Mon, 06 Jan 2025 10:24:26 +0100 Subject: [PATCH v2 2/3] object-file: don't special-case missing source file in collision check Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250106-b4-pks-object-file-racy-collision-check-v2-2-8b3984ecbb18@pks.im> References: <20250106-b4-pks-object-file-racy-collision-check-v2-0-8b3984ecbb18@pks.im> In-Reply-To: <20250106-b4-pks-object-file-racy-collision-check-v2-0-8b3984ecbb18@pks.im> To: git@vger.kernel.org Cc: Jeff King , Junio C Hamano X-Mailer: b4 0.14.2 In 0ad3d65652 (object-file: fix race in object collision check, 2024-12-30) we have started to ignore ENOENT when opening either the source or destination file of the collision check. This was done to handle races more gracefully in case either of the potentially-colliding disappears. The fix is overly broad though: while the destination file may indeed vanish racily, this shouldn't ever happen for the source file, which is a temporary object file (either loose or in packfile format) that we have just created. So if any concurrent process would have removed that temporary file it would indicate an actual issue. Stop treating ENOENT specially for the source file so that we always bubble up this error. Signed-off-by: Patrick Steinhardt --- object-file.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/object-file.c b/object-file.c index e1989236ca87e565dea4d003f57882f257889ecf..acfda5e303659195109c94f5b54b8a081fe92c59 100644 --- a/object-file.c +++ b/object-file.c @@ -1978,8 +1978,7 @@ static int check_collision(const char *source, const char *dest) fd_source = open(source, O_RDONLY); if (fd_source < 0) { - if (errno != ENOENT) - ret = error_errno(_("unable to open %s"), source); + ret = error_errno(_("unable to open %s"), source); goto out; } From patchwork Mon Jan 6 09:24:27 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13927105 Received: from fout-a4-smtp.messagingengine.com (fout-a4-smtp.messagingengine.com [103.168.172.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 211F91D90BE for ; Mon, 6 Jan 2025 09:24:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.147 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736155477; cv=none; b=MNzvpE0ooK9w9VGcE7DkzSdddIgcfqFWGkUsm4SZmBvf4IkdtjJWoMe0Ma7JufXReGiOswXBHrluX+QJS8BFJGBMKjgSCaL8MDKIbHXFFsnN8YnzRY9vv+P6iU7hLZp6p9CGn6ZYcvR0c7CYbsVFJPyNMt8cmszswBE/3cq47b0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736155477; c=relaxed/simple; bh=Rk2OOghvesikhTatPL7PtzcW68K2fL2TN5eP2rIZy/s=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=ltdMD/gV9thwlKcYQTgwQf8BGOhlPmLamGFlIDK7eWxR+fbe8sqC4gxX5EeogdTCl78kZTSPQFdH4tThuvRyn9F/4HGlYVdnuTK4rE6G+3Xj6T1Sy3LgJf9AWq24oh1a5pa7CwwSdclt1B6tS4neKCpBsXcNiU7fAOAS4zO22Lg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=aQdU/Qmk; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=lOnwziNa; arc=none smtp.client-ip=103.168.172.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="aQdU/Qmk"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="lOnwziNa" Received: from phl-compute-12.internal (phl-compute-12.phl.internal [10.202.2.52]) by mailfout.phl.internal (Postfix) with ESMTP id F03301380169; Mon, 6 Jan 2025 04:24:33 -0500 (EST) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-12.internal (MEProxy); Mon, 06 Jan 2025 04:24:33 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1736155473; x=1736241873; bh=N7KFPpnrFwKQWE6KRNGekgnx8PE2LUtmdvOwYkwnNJY=; b= aQdU/QmkDh06mLEIuqJ84KfMFijfLYk/+ho/gKrB0NaX4UfyqLACBRnWq3ZdbPBh W4tNR7QFnMtprP3lAuairdQx1++WUakcvqUA8yDi8r5bq1F3vYXZImM6FCQEwwqS bHXnOQKXlELUAe+JReNTsL+/RZKu/xxnAlqccP+s8IE10q5sWC/4BOkkCoM62Pl6 A2ApdYRUlSqOW11y0snG7UzYIKuZqbLK1KtBnjJ8kaB4SMdd4pLdaPARFoY/GkXe ObK2V9SraLmLBq9VtMbtafia+1wYWlVMoMMMg9bcFQB3pfvipGjU23E6Z5xJGulW YoX27mOPuC/TqyaENz4ieg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1736155473; x= 1736241873; bh=N7KFPpnrFwKQWE6KRNGekgnx8PE2LUtmdvOwYkwnNJY=; b=l OnwziNaHPjBSWcS/o8pmJ9IFIq4LVLgw9QV/W9y8CkkWg37/l3AmysQfH01fJPwU GUx9LgAqpl8wqrDU8XXHjvdRVGZmpwSNYD2iIcXlFuOHKSNaVEbnTZHAEkQORo8A b0YwztleBUVsXsOY30z0DgF2o2c3WihZx/Rc0E6Z+OX88Z4EYxCSmwLbJ7uGhYyN yFWVKxWLD7jRGGytOvEzaz5K37X8mH77XMAe9tBWZHx55Hzq/+bMTU3EoMbLAxFu jTWfMfffDqASH/guhkGXgGHD0OeJUuTN9gNI6mKbkZkpB62I4OlQgoUk91Kky7+i oBp3iOlOkbDowoLhFKqQQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefuddrudegtddgtddvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucenucfjughrpefhfffugg gtgffkfhgjvfevofesthejredtredtjeenucfhrhhomheprfgrthhrihgtkhcuufhtvghi nhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvghrnhepffeuieduje dvkeehuedvkeefffeivdeuleetkeduheejteekgedvudfgtdfgieelnecuvehluhhsthgv rhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimhdpnh gspghrtghpthhtohepfedpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtohepghhithhs thgvrhesphhosghogidrtghomhdprhgtphhtthhopehpvghffhesphgvfhhfrdhnvghtpd hrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrgh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 6 Jan 2025 04:24:33 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id c55cad0a (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 6 Jan 2025 09:24:30 +0000 (UTC) From: Patrick Steinhardt Date: Mon, 06 Jan 2025 10:24:27 +0100 Subject: [PATCH v2 3/3] object-file: retry linking file into place when occluding file vanishes Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250106-b4-pks-object-file-racy-collision-check-v2-3-8b3984ecbb18@pks.im> References: <20250106-b4-pks-object-file-racy-collision-check-v2-0-8b3984ecbb18@pks.im> In-Reply-To: <20250106-b4-pks-object-file-racy-collision-check-v2-0-8b3984ecbb18@pks.im> To: git@vger.kernel.org Cc: Jeff King , Junio C Hamano X-Mailer: b4 0.14.2 Prior to 0ad3d65652 (object-file: fix race in object collision check, 2024-12-30), callers could expect that a successful return from `finalize_object_file()` means that either the file was moved into place, or the identical bytes were already present. If neither of those happens, we'd return an error. Since that commit, if the destination file disappears between our link(3p) call and the collision check, we'd return success without actually checking the contents, and without retrying the link. This solves the common case that the files were indeed the same, but it means that we may corrupt the repository if they weren't (this implies a hash collision, but the whole point of this function is protecting against hash collisions). We can't be pessimistic and assume they're different; that hurts the common case that the mentioned commit was trying to fix. But after seeing that the destination file went away, we can retry linking again. Adapt the code to do so when we see that the destination file has racily vanished. This should generally succeed as we have just observed that the destination file does not exist anymore, except in the very unlikely event that it gets recreated by another concurrent process again. Helped-by: Jeff King Signed-off-by: Patrick Steinhardt --- object-file.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/object-file.c b/object-file.c index acfda5e303659195109c94f5b54b8a081fe92c59..aeca61b8ae3aadecef1ce11306b38e7368af829f 100644 --- a/object-file.c +++ b/object-file.c @@ -1970,6 +1970,8 @@ static void write_object_file_prepare_literally(const struct git_hash_algo *algo hash_object_body(algo, &c, buf, len, oid, hdr, hdrlen); } +#define CHECK_COLLISION_DEST_VANISHED -2 + static int check_collision(const char *source, const char *dest) { char buf_source[4096], buf_dest[4096]; @@ -1986,6 +1988,8 @@ static int check_collision(const char *source, const char *dest) if (fd_dest < 0) { if (errno != ENOENT) ret = error_errno(_("unable to open %s"), dest); + else + ret = CHECK_COLLISION_DEST_VANISHED; goto out; } @@ -2033,8 +2037,11 @@ int finalize_object_file(const char *tmpfile, const char *filename) int finalize_object_file_flags(const char *tmpfile, const char *filename, enum finalize_object_file_flags flags) { - struct stat st; - int ret = 0; + unsigned retries = 0; + int ret; + +retry: + ret = 0; if (object_creation_mode == OBJECT_CREATION_USES_RENAMES) goto try_rename; @@ -2055,6 +2062,8 @@ int finalize_object_file_flags(const char *tmpfile, const char *filename, * left to unlink. */ if (ret && ret != EEXIST) { + struct stat st; + try_rename: if (!stat(filename, &st)) ret = EEXIST; @@ -2070,9 +2079,17 @@ int finalize_object_file_flags(const char *tmpfile, const char *filename, errno = saved_errno; return error_errno(_("unable to write file %s"), filename); } - if (!(flags & FOF_SKIP_COLLISION_CHECK) && - check_collision(tmpfile, filename)) + if (!(flags & FOF_SKIP_COLLISION_CHECK)) { + ret = check_collision(tmpfile, filename); + if (ret == CHECK_COLLISION_DEST_VANISHED) { + if (retries++ > 5) + return error(_("unable to write repeatedly vanishing file %s"), + filename); + goto retry; + } + else if (ret) return -1; + } unlink_or_warn(tmpfile); }