Re: File locks for data directory lockfile in the context of Linux namespaces - Mailing list pgsql-hackers

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)))