From: Zsolt Parragi <zsolt.parragi@percona.com>
In-Reply-To: <c5jj2zvhnh36xfvqkcogrvk5fkcut3un5q5hibozfm62h6t4kp@sdlorh7uaf2s>
References: <z2u3jtjzhqofqhrjvfgkwl4cczhpqibz3t47gpx7fbe7ceaihk@i5mveu2wfpxn>
 <45swup77t7utfy33lu47mvkb4exzm3oeakz25abqxuvmsyzt3c@5umrpfgc3ypg>
 <178066304625.594057.12185296716401427152.pgcf@coridan.postgresql.org>
 <54uh7vhavffktp72dmjm7ms26lntxacnvpwuaitiez4izugdi6@emwc2yrpqwk2>
 <xhzl7ll7fwmlecg4htsgvr3fcwcec4cvugqebphofvcz4srkkn@vpwa2zgswuwy>
 <178332300475.2568959.4097339122938147476.pgcf@coridan.postgresql.org>
 <c5jj2zvhnh36xfvqkcogrvk5fkcut3un5q5hibozfm62h6t4kp@sdlorh7uaf2s>
MIME-Version: 1.0
Date: Mon, 6 Jul 2026 15:07:35 -0700
Message-ID: 
 <CAN4CZFMepY-70PqQjefnFFR1VFjvos5EnkpU75ho9bvQj=KXoQ@mail.gmail.com>
Subject: Re: File locks for data directory lockfile in the context of Linux
 namespaces
To: pgsql-hackers@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAN4CZFMepY-70PqQjefnFFR1VFjvos5EnkpU75ho9bvQj%3DKXoQ%40mail.gmail.com>
Precedence: bulk

Hello!

+		if (errno == EAGAIN)
+			ereport(FATAL,
+					(errcode(ERRCODE_LOCK_FILE_EXISTS),

According to fcntl.2, this should handle both EACCESS and EAGAIN:

ERRORS
       EACCES or EAGAIN
              Operation is prohibited by locks held by other processes.

+static int
+OFDLockFile(int fd, const char *filename)
+...
+	else
+		return dup(fd);

Isn't this missing an FD_CLOEXEC, so that launched processes doesn't
inherit it and keep the lock open possibly longer than needed?

Also, shouldn't the code verify the result of dup? (!= -1 / errno)

+		flock_fd = OFDLockFile(fd, filename);

Can't we leak flock_fd in the stale path?

+		 * Close the file descriptor, which keeps the open file description
+		 * lock.
+		 */
+		if (lock_file->fd > 0)
+			close(lock_file->fd);

Shouldn't this check for >= 0?

+			elog(WARNING, "Failed locking file \"%s\", %m", filename);

This probably should be:

ereport(WARNING, (errcode_for_file_access(), errmsg("could not lock
file \"%s\": %m", filename)))