From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Michal Nazarewicz Newsgroups: gmane.emacs.bugs Subject: bug#72641: 31.0.50; "Unlocking file: Invalid argument" when deleting lock file on network file system Date: Fri, 16 Aug 2024 02:59:32 +0200 Message-ID: <0aecnuy9e9nb2pjfpz4dpvqk@mina86.com> References: <867cfujge6.fsf@gnu.org> <86fruhiwt0.fsf@gnu.org> <868r09ivqn.fsf@gnu.org> <867cftiprt.fsf@gnu.org> <86msoph6wt.fsf@gnu.org> <2+lcnmedng9le3pwfn0gc79m@mina86.com> <86a5hd7o4t.fsf@gnu.org> <+b8wcnteufggsda3gtf7ioja@mina86.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="24039"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 72641@debbugs.gnu.org To: Paul Eggert , Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Aug 16 03:01:51 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1selLi-000663-V5 for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 16 Aug 2024 03:01:51 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1selLS-00006x-TD; Thu, 15 Aug 2024 21:01:34 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1selLL-00006Y-7T for bug-gnu-emacs@gnu.org; Thu, 15 Aug 2024 21:01:29 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1selLJ-0001ea-KO for bug-gnu-emacs@gnu.org; Thu, 15 Aug 2024 21:01:26 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=MIME-Version:Date:References:In-Reply-To:From:To:Subject; bh=M7zqua/cRT/hMpO5WhKnqBw78NOdBzG9/4ezoCxr/NU=; b=dsBx4SXN4exuGYPNp+CYAqVn/mbqjseDRdl2e7elhRcbM/QmMwFuTnrnwpW1/kfLqHWPQTrqI8XcvAIuadjfkI8gbeLWATK0nFLtRdfvSQ29IsKu67Bos0i3TCdiVNdPzX5bWPRbcF1esigke20n6b/YlkTIwhRLGNeCM7DRwrvWQihcnBkXlJCIZukiWWgzsstiSaUmGDGu6/xeDRl0NG20AH+ArXWKJkQwiFhuyZZHOCLtId5s6F391ZVsps3YCdhcHmWNiHK/wAaKmj1pJdewG/tE7F3o3ihvNEPqa+aTeulvinljQIa7W30CC4Wf3Ql4ugsl9o2ee0XQSE8Hmw==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1selLt-00089k-Si for bug-gnu-emacs@gnu.org; Thu, 15 Aug 2024 21:02:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Michal Nazarewicz Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 16 Aug 2024 01:02:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 72641 X-GNU-PR-Package: emacs Original-Received: via spool by 72641-submit@debbugs.gnu.org id=B72641.172377009131306 (code B ref 72641); Fri, 16 Aug 2024 01:02:01 +0000 Original-Received: (at 72641) by debbugs.gnu.org; 16 Aug 2024 01:01:31 +0000 Original-Received: from localhost ([127.0.0.1]:49979 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1selLO-00088r-RD for submit@debbugs.gnu.org; Thu, 15 Aug 2024 21:01:31 -0400 Original-Received: from mail-ed1-f54.google.com ([209.85.208.54]:41287) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1selLC-00088K-SV for 72641@debbugs.gnu.org; Thu, 15 Aug 2024 21:01:27 -0400 Original-Received: by mail-ed1-f54.google.com with SMTP id 4fb4d7f45d1cf-5bec50de782so153871a12.1 for <72641@debbugs.gnu.org>; Thu, 15 Aug 2024 18:00:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1723769975; x=1724374775; darn=debbugs.gnu.org; h=content-transfer-encoding:mime-version:message-id:date:face :references:in-reply-to:subject:cc:to:from:sender:from:to:cc:subject :date:message-id:reply-to; bh=M7zqua/cRT/hMpO5WhKnqBw78NOdBzG9/4ezoCxr/NU=; b=Mmt22tCvtUZEgJEzzqb0UaOml9wo7ghUmMa6xKszh4fu5iQSA3S8vvHIw9vOJ5b12D /SJW6pVoWxR9u14GyYHF6BRpfXwPh8fSSk6Cc8fYX0rxciuPGvCWSea+LI6pPJvewPNW FFL3WkZEu22aGsCYVpJhrjRrKpbxx2dZiQLqNzbecxNor3/0gn0StiZCxSGN2It/xJRr gbVoxv19ga9+DcES1HnbKiLlrbqWbRTQorks5Dmz6KROyVoksfD2LDX8a498QBFiEV7a NFwVmkIdiif2nCMFNaVvmId2nkvsznw1+quPM5dGG+RMRroTLrd9EoQ/WOjL9g4INwKr gL4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723769975; x=1724374775; h=content-transfer-encoding:mime-version:message-id:date:face :references:in-reply-to:subject:cc:to:from:sender:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=M7zqua/cRT/hMpO5WhKnqBw78NOdBzG9/4ezoCxr/NU=; b=wzoDdGkszoULSx4Mf6OrMYilVmEDMWxqpI86+hjRK1ZkM5z4TzHNbhzJKymwn9rAM9 AK5O1oPc5Q/a9gYwmAM+BToHhPVtWK0qDt7JAIOrg0z9wEUypUhXtbJX0+0z07IEmWaL wOQT6oLiUjeQhujXtr3oBQn/dDPSyIJtnAfu2eF/LKqnlzqpUjXacYv/mWDacbXc4eDA nhhgVyJjGCXiD/KIkVbZdhPRhXwR6jOjP2Bo78voDlFHbxq3cCZukCIba3QJs1u9VHAp rfwI8MF4NAw5haZsRNESdn6u6T4JEhkWXmSz4MD6IzxaGbvh48rF95g56kWr0kEeq15V 1Lcg== X-Gm-Message-State: AOJu0YwwvXiHumyl72H3jLjQxPqrkO8ECl6tbJCCqxnecxHx4PoOp259 nrK9snn/k6asQ4QsGGIhll1cR252+nLGyCYvEexweBuS6dvvX6C6ZJiykg== X-Google-Smtp-Source: AGHT+IEyfcUtjejOLIsviM+S6U0eRzeccGPgxrnoHRz9xpkbWzCpreHnrS9g4mSOEgTRhJ1STgk+vA== X-Received: by 2002:a17:907:e8c:b0:a80:a37f:c303 with SMTP id a640c23a62f3a-a8392c20eb9mr56873466b.4.1723769974164; Thu, 15 Aug 2024 17:59:34 -0700 (PDT) Original-Received: from erwin (87-205-2-211.static.ip.netia.com.pl. [87.205.2.211]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a838396d3adsm173350866b.218.2024.08.15.17.59.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Aug 2024 17:59:33 -0700 (PDT) In-Reply-To: Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAAJFBMVEWjgIPUupJ7V0jLrom4gmjPs42bY0MdFRLHgE5UPDCbfGm9mH6qmkAJAAACNUlEQVQ4y23SMW/aQBQHcKtb2Zx0abZeIxuTCSELJVmiinboRpGHJktloROQzUQcB2vUXFe35XBYUicRAiYUVSjfru/d+QwlnDz5p/97z+dnVcw5WVQ2zxpcdrQTTiIin3bB6lcaTnaBO5c8eoJG2yBl5El+Ob3fglMW3RUIkQ9xxQ8UBEFQafiVZ/5wZqWExM+LvwEegAY856xZO7MgQkq3jxpO56GXxO5VDQAjpVmWqPLYoSGvIahIuvAVHLeEEDd9DRYAkUcKzscAUgCQ0hwSKBOExuh7kvQQUskY4yjsHqeaCfFjLK6x1AzeeyiRD/C1JW5aGqw5IS5mZre+FVz0RVtoUGPJmBCP/4QPJGKUA1oEMJwAHIv+JljYZYCJi1FRQS2DaUw6sgsQTMfb0Fz9RvjyP3iR01x9RGj0N6HgcefS819CSjidDwIFPSFEDgRg1tUw2oBXAB33T2DV63XbM6AqhZzysm3ZcKZrKBAn5O7Q1rAqGoBKNGRsmUG1n0NKaJtd2RnscTWvnom6fGngDaNrCNl728A+c2gLQP8PFpkEjNWjlGqQfPEhWRqoInSyi2p+drsGDjhAeAeSQI8kfjSwbwCn8gblvIftInzT30HdQT2HlU4gwCbLSQ7VFqVEA0RCtjTwtnjdLlkasNjQwF7x02F2iQSL8XIGB8 VQ71WBKHHf5XCIgLUUtIcGSBPhdZIQorpEBrxsqT3GYgRpmicIqrsEgSZm3FG+iJbHYid0/wGj+iTGCXRsqQAAAABJRU5ErkJggg== X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:290189 Archived-At: On Thu, Aug 15 2024, Paul Eggert wrote: > That being said, I think Emacs can ignore and remove bad lock files=20 > without introducing more races. I installed the attached into the master= =20 > branch and it works for me on your test case (which I introduced=20 > artificially on GNU/Linux). Please give it a try. With Emacs from current master, if I open a file, edit it and then kill the buffer without saving the changes, the lock file is deleted. However, if I save the file (be it by save-buffer or by killing the buffer and picking save option), the lock file remains. I didn=E2=80=99t fully track what is actually happening. It looks like sav= ing the buffer results in the following: - lock_file is called - it calls current_lock_owner - which deletes the lock file - and now lock_file creates new lock file - unlock_file is called - it calls current_lock_owner - which return ENOENT for some reason - the lock file is left alone =20 In desperation I=E2=80=99ve tried attached patch (it adds `if (!lfinfolen) return I_OWN_IT`; big diff is because than `if (lfinfolen)` check can be removed and code dedented) and it worked. Maybe this is a better approach? Because currently lock_file will delete the lock file and then create an empty file. With the below file, lock_file will just notice file is there and do nothing. >From be05054ae47e74192bb3551e83d4afb2ff41f888 Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Fri, 16 Aug 2024 02:49:55 +0200 Subject: [PATCH] Treat empty lock files as owned by us (bug#72641) * src/filelock.c (current_lock_owner): Rather then deleting empty lock files, treat them as if they were owned by us. Previous commit which deleted the lock file instead resulted in stale lock file remaining when saving a file. --- src/filelock.c | 185 ++++++++++++++++++++++++------------------------- 1 file changed, 92 insertions(+), 93 deletions(-) diff --git a/src/filelock.c b/src/filelock.c index 1ae57dc7344..f9aac0dc5c5 100644 --- a/src/filelock.c +++ b/src/filelock.c @@ -397,110 +397,109 @@ current_lock_owner (lock_info_type *owner, Lisp_Obj= ect lfname) if (lfinfolen < 0) return errno =3D=3D ENOENT || errno =3D=3D ENOTDIR ? 0 : errno; =20 + /* The lock file is empty which may be due to buggy file system, e.g., + . Treat it as us holding the lock. */ + if (!lfinfolen) + return I_OWN_IT; + /* If the lock file seems valid, return a value based on its contents. = */ - if (lfinfolen) + if (MAX_LFINFO < lfinfolen) + return ENAMETOOLONG; + owner->user[lfinfolen] =3D 0; + + /* Parse USER@HOST.PID:BOOT_TIME. If can't parse, return EINVAL. */ + /* The USER is everything before the last @. */ + char *at =3D memrchr (owner->user, '@', lfinfolen); + if (!at) + return EINVAL; + owner->at =3D at; + char *dot =3D strrchr (at, '.'); + if (!dot) + return EINVAL; + owner->dot =3D dot; + + /* The PID is everything from the last '.' to the ':' or equivalent. */ + if (! integer_prefixed (dot + 1)) + return EINVAL; + errno =3D 0; + intmax_t pid =3D strtoimax (dot + 1, &owner->colon, 10); + if (errno =3D=3D ERANGE) + pid =3D -1; + + /* After the ':' or equivalent, if there is one, comes the boot time. */ + intmax_t boot_time; + char *boot =3D owner->colon + 1, *lfinfo_end; + switch (owner->colon[0]) { - if (MAX_LFINFO < lfinfolen) - return ENAMETOOLONG; - owner->user[lfinfolen] =3D 0; - - /* Parse USER@HOST.PID:BOOT_TIME. If can't parse, return EINVAL. */ - /* The USER is everything before the last @. */ - char *at =3D memrchr (owner->user, '@', lfinfolen); - if (!at) - return EINVAL; - owner->at =3D at; - char *dot =3D strrchr (at, '.'); - if (!dot) - return EINVAL; - owner->dot =3D dot; + case 0: + boot_time =3D 0; + lfinfo_end =3D owner->colon; + break; =20 - /* The PID is everything from the last '.' to the ':' or equivalent.= */ - if (! integer_prefixed (dot + 1)) + case '\357': + /* Treat "\357\200\242" (U+F022 in UTF-8) like ":" (Bug#24656). + This works around a bug in the Linux CIFS kernel client, which can + mistakenly transliterate ':' to U+F022 in symlink contents. + See . */ + if (! (boot[0] =3D=3D '\200' && boot[1] =3D=3D '\242')) return EINVAL; - errno =3D 0; - intmax_t pid =3D strtoimax (dot + 1, &owner->colon, 10); - if (errno =3D=3D ERANGE) - pid =3D -1; - - /* After the ':' or equivalent, if there is one, comes the boot time= . */ - intmax_t boot_time; - char *boot =3D owner->colon + 1, *lfinfo_end; - switch (owner->colon[0]) - { - case 0: - boot_time =3D 0; - lfinfo_end =3D owner->colon; - break; - - case '\357': - /* Treat "\357\200\242" (U+F022 in UTF-8) like ":" (Bug#24656). - This works around a bug in the Linux CIFS kernel client, which can - mistakenly transliterate ':' to U+F022 in symlink contents. - See . */ - if (! (boot[0] =3D=3D '\200' && boot[1] =3D=3D '\242')) - return EINVAL; - boot +=3D 2; - FALLTHROUGH; - case ':': - if (! integer_prefixed (boot)) - return EINVAL; - boot_time =3D strtoimax (boot, &lfinfo_end, 10); - break; - - default: - return EINVAL; - } - if (lfinfo_end !=3D owner->user + lfinfolen) + boot +=3D 2; + FALLTHROUGH; + case ':': + if (! integer_prefixed (boot)) return EINVAL; + boot_time =3D strtoimax (boot, &lfinfo_end, 10); + break; =20 - char *linkhost =3D at + 1; - ptrdiff_t linkhostlen =3D dot - linkhost; - Lisp_Object system_name =3D Fsystem_name (); - /* If `system-name' returns nil, that means we're in a - --no-build-details Emacs, and the name part of the link (e.g., - .#test.txt -> larsi@.118961:1646577954) is an empty string. */ - bool on_current_host; - if (NILP (system_name)) - on_current_host =3D linkhostlen =3D=3D 0; - else - { - on_current_host =3D linkhostlen =3D=3D SBYTES (system_name); - if (on_current_host) - { - /* Protect against the extremely unlikely case of the host - name containing '@'. */ - char *sysname =3D SSDATA (system_name); - for (ptrdiff_t i =3D 0; i < linkhostlen; i++) - if (linkhost[i] !=3D (sysname[i] =3D=3D '@' ? '-' : sysname[i])) - { - on_current_host =3D false; - break; - } - } - } - if (!on_current_host) + default: + return EINVAL; + } + if (lfinfo_end !=3D owner->user + lfinfolen) + return EINVAL; + + char *linkhost =3D at + 1; + ptrdiff_t linkhostlen =3D dot - linkhost; + Lisp_Object system_name =3D Fsystem_name (); + /* If `system-name' returns nil, that means we're in a + --no-build-details Emacs, and the name part of the link (e.g., + .#test.txt -> larsi@.118961:1646577954) is an empty string. */ + bool on_current_host; + if (NILP (system_name)) + on_current_host =3D linkhostlen =3D=3D 0; + else + { + on_current_host =3D linkhostlen =3D=3D SBYTES (system_name); + if (on_current_host) { - /* Not on current host. If we wanted to support the check for - stale locks on remote machines, here's where we'd do it. */ - return ANOTHER_OWNS_IT; + /* Protect against the extremely unlikely case of the host + name containing '@'. */ + char *sysname =3D SSDATA (system_name); + for (ptrdiff_t i =3D 0; i < linkhostlen; i++) + if (linkhost[i] !=3D (sysname[i] =3D=3D '@' ? '-' : sysname[i])) + { + on_current_host =3D false; + break; + } } + } + if (!on_current_host) + { + /* Not on current host. If we wanted to support the check for + stale locks on remote machines, here's where we'd do it. */ + return ANOTHER_OWNS_IT; + } =20 - if (pid =3D=3D getpid ()) - return I_OWN_IT; + if (pid =3D=3D getpid ()) + return I_OWN_IT; =20 - if (VALID_PROCESS_ID (pid) - && ! (kill (pid, 0) < 0 && errno !=3D EPERM) - && (boot_time =3D=3D 0 - || within_one_second (boot_time, get_boot_sec ()))) - return ANOTHER_OWNS_IT; - } + if (VALID_PROCESS_ID (pid) + && ! (kill (pid, 0) < 0 && errno !=3D EPERM) + && (boot_time =3D=3D 0 + || within_one_second (boot_time, get_boot_sec ()))) + return ANOTHER_OWNS_IT; =20 - /* The owner process is dead or has a strange pid, or the lock file is e= mpty. - Try to zap the lockfile. If the lock file is empty, this assumes - the file system is buggy, e.g., . - Emacs never creates empty lock files even temporarily, so removing - an empty lock file should be harmless. */ + /* The owner process is dead or has a strange pid. + Try to zap the lockfile. */ return emacs_unlink (SSDATA (lfname)) < 0 ? errno : 0; } =20 --=20 2.43.0