[PATCH 0/1] External fix for sporadic schroot race issue

[Anton Mikanovich]

After some experimenting with locking inside and outside of schroot and
sbuild tools I've finally found the way how to made almost the same
locking in Isar only without much build speed influence.

The idea is to cover all separate schroot calls with the lock based on
what type of the call it is:
- Session begin and any commands in already present session will use
the lock in shared mode, so multiple executions possible.
- Session end which can remove config files and cause race scenario
will use the lock in exclusive mode, so it will wait for the time no
other schroot instances running.

Luckly we always use schroot with separate session create/end commands
even inside sbuild.
The only thing needed is to put a little script into the location
inserted to PATH.

This patch is just a copy of RFC was sent previously with no changes.
It was tested on our CI and now is ready to be merged.

Anton Mikanovich (1):
meta: Protect schroot config management

meta/classes/dpkg.bbclass | 3 +++
meta/classes/sbuild.bbclass | 6 ++++++
scripts/schroot | 43 +++++++++++++++++++++++++++++++++++++
3 files changed, 52 insertions(+)
create mode 100755 scripts/schroot

--
2.34.1

[Anton Mikanovich]

As schroot itself is not thread safe and can fail in case of reading
chroot/session config files when other instance removing those files,
we need to add external locking for race protection.
Run schroot through flock protected script provided via PATH by
default. Also protect with the same lock removing of configs.

Signed-off-by: Anton Mikanovich <ami...@ilbers.de>
---

meta/classes/dpkg.bbclass | 3 +++
meta/classes/sbuild.bbclass | 6 ++++++
scripts/schroot | 43 +++++++++++++++++++++++++++++++++++++
3 files changed, 52 insertions(+)
create mode 100755 scripts/schroot

diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index ef85890a..64404103 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -96,6 +96,9 @@ dpkg_runbuild() {

export SBUILD_CONFIG="${SBUILD_CONFIG}"

+ # Provide locking filter for schroot
+ sbuild_add_env_filter "PATH"
+
for envvar in http_proxy HTTP_PROXY https_proxy HTTPS_PROXY \
ftp_proxy FTP_PROXY no_proxy NO_PROXY; do
sbuild_add_env_filter "$envvar"
diff --git a/meta/classes/sbuild.bbclass b/meta/classes/sbuild.bbclass
index f68e8735..1ab72aad 100644
--- a/meta/classes/sbuild.bbclass
+++ b/meta/classes/sbuild.bbclass
@@ -14,6 +14,9 @@ SCHROOT_CONF_FILE ?= "${SCHROOT_CONF}/chroot.d/${SBUILD_CHROOT}"

SBUILD_CONFIG="${WORKDIR}/sbuild.conf"

+# Lockfile available for all the users
+SCHROOT_LOCKFILE = "/tmp/schroot.lock"
+
schroot_create_configs() {
mkdir -p "${TMPDIR}/schroot-overlay"
echo "Creating ${SCHROOT_CONF_FILE}"
@@ -54,6 +57,8 @@ EOSUDO
}

schroot_delete_configs() {
+ (flock -x 9
+ set -e
sudo -s <<'EOSUDO'
set -e
if [ -d "${SBUILD_CONF_DIR}" ]; then
@@ -63,6 +68,7 @@ schroot_delete_configs() {
echo "Removing ${SCHROOT_CONF_FILE}"
rm -f "${SCHROOT_CONF_FILE}"
EOSUDO
+ ) 9>"${SCHROOT_LOCKFILE}"
}

sbuild_add_env_filter() {
diff --git a/scripts/schroot b/scripts/schroot
new file mode 100755
index 00000000..f5320a6a
--- /dev/null
+++ b/scripts/schroot
@@ -0,0 +1,43 @@
+#!/bin/bash
+#
+# This software is a part of ISAR.
+# Copyright (C) 2024 ilbers GmbH
+#
+# SPDX-License-Identifier: MIT
+
+set -e
+
+# Save command line
+OPTS=("$@")
+
+# Analyze used flags
+while [ $# -gt 0 ]
+do
+ key="$1"
+
+ case $key in
+ -b|--begin-session)
+ BEGIN="1"
+ ;;
+ -r|--run-session)
+ RUN="1"
+ ;;
+ -e|--end-session)
+ END="1"
+ ;;
+ esac
+
+ shift
+done
+
+# Use exclusive lock for configs rm, shared for any other calls
+TYPE="-s"
+if [ "$END" == "1" ]; then
+ TYPE="-x"
+fi
+
+# A place for lock available for all the users
+LOCKDIR="/tmp"
+
+# Run schroot protected with lock
+flock $TYPE $LOCKDIR/schroot.lock /usr/bin/schroot "${OPTS[@]}"
--
2.34.1

[Uladzimir Bely]

Applied to next.

--
Best regards,
Uladzimir.