[PATCH 0/4] Fix various race conditions on multiconfig

[PATCH 0/4] Fix various race conditions on multiconfig

['Felix Moessbauer' via isar-users]

When running under multiconfig, various race conditions can occur due
to simultaneous access to the isar-apt. We fix these by ensuring
that the deployment of the source archive and the .debs is idempotent.

While investigating this, we noticed that the existing deb_clean no
longer removes the artifacts from the apt-cache, as it reads the "what
to clean" from a dir that is previously removed by the internal
clean logic of the sstate. This bug is fixed as well (p2).

Note, that the root cause of the races has been found by an LLM,
which had access to the build log of a failed execution.

Best regards,
Felix Moessbauer
Siemens AG

Felix Moessbauer (4):
  fix(dpkg-build): make sstate updates of DEPLOY_DIR_DEB atomic
  fix(dpkg-build): clean isar-apt by source package and arch
  fix(dpkg-build): scan DEPLOY_DIR_DEB only once in do_deploy_deb
  fix(dpkg-source): make sstate updates of DEPLOY_DIR_SRC atomic

 meta/classes-recipe/dpkg-base.bbclass   | 53 +++++++++++++++++++------
 meta/classes-recipe/dpkg-source.bbclass | 35 ++++++++++++----
 meta/classes-recipe/repository.bbclass  | 22 ++++++++++
 3 files changed, 89 insertions(+), 21 deletions(-)

-- 
2.55.0

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/isar-users/20260910140201.250854-1-felix.moessbauer%40siemens.com.

[PATCH 1/4] fix(dpkg-build): make sstate updates of DEPLOY_DIR_DEB atomic

['Felix Moessbauer' via isar-users]

DEPLOY_DIR_DEB is qualified by DISTRO, DISTRO_ARCH and PN, but not by
MACHINE. All multiconfigs sharing DISTRO and DISTRO_ARCH therefore run
do_dpkg_build and do_deploy_deb of the very same recipe on one directory,
concurrently and without any mutual exclusion. testsuite's test_cross for
example builds mc:qemuarm-bookworm and mc:nanopi-neo-efi-bookworm, both
armhf, so both provide hello-isar and example-module-armmp.

sstate_clean() unlinks the debs listed in the manifest before
sstate_install() puts them back. A do_deploy_deb running in parallel can
therefore observe DEPLOY_DIR_DEB partially populated, remove exactly the
packages that are still there from isar-apt and add back only those, so
the remaining ones silently disappear from the repository.

Take a dedicated lock in both sstate_clean() and sstate_install() of
do_dpkg_build, and in do_deploy_deb, via the sstate-lockfile task flag.
DEPLOY_DIR_DEB is then either complete or empty for do_deploy_deb, never
partial. The lock lives next to, not inside, DEPLOY_DIR_DEB so that
sstate_clean_manifest() cannot sweep it away.

Fixes: 1182a45b ("fix(dpkg-build): deploy debs via shared sstate dir")
Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
---
 meta/classes-recipe/dpkg-base.bbclass | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/meta/classes-recipe/dpkg-base.bbclass b/meta/classes-recipe/dpkg-base.bbclass
index 4a81787e..164f247e 100644
--- a/meta/classes-recipe/dpkg-base.bbclass
+++ b/meta/classes-recipe/dpkg-base.bbclass
@@ -18,6 +18,13 @@ inherit essential
 # WORKDIR lifecycle.
 DEPLOYDIR = "${WORKDIR}/deploy"
 DEPLOY_DIR_DEB = "${DEPLOY_DIR}/isar-deb/${DISTRO}-${DISTRO_ARCH}/${PN}"
+# DEPLOY_DIR_DEB is not qualified by MACHINE, so all multiconfigs sharing
+# DISTRO and DISTRO_ARCH run do_dpkg_build and do_deploy_deb of the same recipe
+# on it, concurrently. Serialize the sstate clean/install of do_dpkg_build
+# against do_deploy_deb, so that the latter never sees a partially populated
+# directory. Keep the lock next to, not inside, DEPLOY_DIR_DEB so that
+# sstate_clean_manifest() cannot sweep it away.
+DEPLOY_DIR_DEB_LOCK = "${DEPLOY_DIR}/isar-deb/${DISTRO}-${DISTRO_ARCH}/${PN}.lock"
 
 DEPENDS ?= ""
 RPROVIDES ?= "${PROVIDES}"
@@ -207,6 +214,7 @@ python do_dpkg_build() {
 do_dpkg_build[cleandirs] = "${DEPLOYDIR}"
 do_dpkg_build[sstate-inputdirs] = "${DEPLOYDIR}"
 do_dpkg_build[sstate-outputdirs] = "${DEPLOY_DIR_DEB}"
+do_dpkg_build[sstate-lockfile] = "${DEPLOY_DIR_DEB_LOCK}"
 do_dpkg_build[network] = "${TASK_USE_NETWORK_AND_SUDO}"
 do_dpkg_build[depends] = "${SCHROOT_DEP} base-apt:do_cache isar-apt:do_cache_config"
 do_dpkg_build[postfuncs] += "dpkg_collect_debs"
@@ -232,7 +240,9 @@ deb_clean() {
         done
     fi
 }
-# the clean function modifies isar-apt
+# the clean function modifies isar-apt. Do not add DEPLOY_DIR_DEB_LOCK here:
+# CLEANFUNCS also runs sstate_cleanall(), which takes that lock itself, and
+# flock() would deadlock on the nested acquisition.
 do_clean[lockfiles] = "${REPO_ISAR_DIR}/isar.lock"
 do_clean[network] = "${TASK_USE_SUDO}"
 
@@ -249,7 +259,7 @@ addtask deploy_deb after do_dpkg_build before do_build
 do_deploy_deb[deptask] = "do_deploy_deb"
 do_deploy_deb[rdeptask] = "do_deploy_deb"
 do_deploy_deb[depends] += "isar-apt:do_cache_config"
-do_deploy_deb[lockfiles] = "${REPO_ISAR_DIR}/isar.lock"
+do_deploy_deb[lockfiles] = "${REPO_ISAR_DIR}/isar.lock ${DEPLOY_DIR_DEB_LOCK}"
 do_deploy_deb[dirs] = "${S} ${DEPLOY_DIR_DEB}"
 
 python do_devshell() {
-- 
2.55.0

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/isar-users/20260910140201.250854-2-felix.moessbauer%40siemens.com.

[PATCH 2/4] fix(dpkg-build): clean isar-apt by source package and arch

['Felix Moessbauer' via isar-users]

deb_clean() removed the packages from isar-apt by iterating the .deb
files in DEPLOY_DIR_DEB. Since do_dpkg_build updates DEPLOY_DIR_DEB via
sstate, its sstate_clean() empties the directory before deb_clean() runs,
so the scan finds nothing and stale packages are left behind in isar-apt.

Remove the packages by their source package name and architecture
instead, using reprepro's removefilter. This no longer depends on the
.deb files being present, is idempotent, and also copes with binary
packages that were split off or renamed between builds.

Fixes: 1182a45b ("fix(dpkg-build): deploy debs via shared sstate dir")
Reported-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
---
 meta/classes-recipe/dpkg-base.bbclass  | 22 +++++++++++++++-------
 meta/classes-recipe/repository.bbclass | 22 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/meta/classes-recipe/dpkg-base.bbclass b/meta/classes-recipe/dpkg-base.bbclass
index 164f247e..66f4d177 100644
--- a/meta/classes-recipe/dpkg-base.bbclass
+++ b/meta/classes-recipe/dpkg-base.bbclass
@@ -231,14 +231,22 @@ addtask dpkg_build_setscene
 
 CLEANFUNCS += "deb_clean"
 
+# Architectures under which this recipe's binary packages end up in the repo:
+# the concrete build arch (PACKAGE_ARCH resolves DPKG_ARCH=any), plus "all"
+# when this is the build that also produces the arch-independent packages.
+def deb_clean_archs(d):
+    dpkg_arch = d.getVar('DPKG_ARCH') or 'any'
+    package_arch = d.getVar('PACKAGE_ARCH')
+    host_arch = d.getVar('HOST_ARCH')
+    archs = [package_arch if dpkg_arch == 'any' else dpkg_arch]
+    if package_arch == host_arch and 'all' not in archs:
+        archs.append('all')
+    return ' '.join(a for a in archs if a)
+
 deb_clean() {
-    DEBS=$( find ${DEPLOY_DIR_DEB} -maxdepth 1 -name "*.deb" || [ ! -d ${S} ] )
-    if [ -n "${DEBS}" ]; then
-        for d in ${DEBS}; do
-            repo_del_package "${REPO_ISAR_DIR}"/"${DISTRO}" \
-                "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" "${d}"
-        done
-    fi
+    repo_del_by_source "${REPO_ISAR_DIR}"/"${DISTRO}" \
+        "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" \
+        "${BPN}" ${@deb_clean_archs(d)}
 }
 # the clean function modifies isar-apt. Do not add DEPLOY_DIR_DEB_LOCK here:
 # CLEANFUNCS also runs sstate_cleanall(), which takes that lock itself, and
diff --git a/meta/classes-recipe/repository.bbclass b/meta/classes-recipe/repository.bbclass
index a2061100..d255d1c8 100644
--- a/meta/classes-recipe/repository.bbclass
+++ b/meta/classes-recipe/repository.bbclass
@@ -129,6 +129,28 @@ repo_del_package() {
     repo_set_release_date "${dir}" "${codename}"
 }
 
+repo_del_by_source() {
+    local dir="$1"
+    local dbdir="$2"
+    local codename="$3"
+    local source="$4"
+    shift 4
+
+    if [ -n "${GNUPGHOME}" ]; then
+        export GNUPGHOME="${GNUPGHOME}"
+    fi
+    local arch_filter=""
+    local arch
+    for arch in "$@"; do
+        [ -n "${arch_filter}" ] && arch_filter="${arch_filter}|"
+        arch_filter="${arch_filter}Architecture (= ${arch})"
+    done
+    reprepro -b "${dir}" --dbdir "${dbdir}" -C main \
+        removefilter "${codename}" \
+        '$Source (= '"${source}"'), ('"${arch_filter}"'), $PackageType (= deb)'
+    repo_set_release_date "${dir}" "${codename}"
+}
+
 repo_contains_package() {
     local dir="$1"
     local dbdir="$2"
-- 
2.55.0

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/isar-users/20260910140201.250854-3-felix.moessbauer%40siemens.com.

[PATCH 3/4] fix(dpkg-build): scan DEPLOY_DIR_DEB only once in do_deploy_deb

['Felix Moessbauer' via isar-users]

do_deploy_deb removed the packages from isar-apt via deb_clean() and then
scanned DEPLOY_DIR_DEB to add them back. DEPLOY_DIR_DEB is shared by all
multiconfigs with the same DISTRO and DISTRO_ARCH, and is transiently
empty while do_dpkg_build of one of them rebuilds the recipe, between
sstate_clean() in its prefunc and sstate_install() in its postfunc. A
do_deploy_deb of another multiconfig hitting that window removed the
package from isar-apt and added nothing back, while still reporting
success. The package was then missing from isar-apt although present in
DEPLOY_DIR_DEB, and do_rootfs_install failed to install it much later:

  E: Unable to locate package example-module-armmp

Scan DEPLOY_DIR_DEB once and skip the update entirely when it is empty,
leaving isar-apt untouched instead of clearing it. That is safe: the
multiconfig doing the rebuild runs its own do_deploy_deb once
do_dpkg_build completed.

Fixes: 1182a45b ("fix(dpkg-build): deploy debs via shared sstate dir")
Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
---
 meta/classes-recipe/dpkg-base.bbclass | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/meta/classes-recipe/dpkg-base.bbclass b/meta/classes-recipe/dpkg-base.bbclass
index 66f4d177..fbb952bd 100644
--- a/meta/classes-recipe/dpkg-base.bbclass
+++ b/meta/classes-recipe/dpkg-base.bbclass
@@ -255,12 +255,21 @@ do_clean[lockfiles] = "${REPO_ISAR_DIR}/isar.lock"
 do_clean[network] = "${TASK_USE_SUDO}"
 
 do_deploy_deb() {
-    deb_clean
+    # Removal is by source name and architecture (deb_clean), so a single scan
+    # of DEPLOY_DIR_DEB is only needed for the addition below.
     debs=$(find ${DEPLOY_DIR_DEB} -maxdepth 1 -name '*.deb')
-    if [ -n "${debs}" ]; then
-        repo_add_packages "${REPO_ISAR_DIR}"/"${DISTRO}" \
-            "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" ${debs}
+    if [ -z "${debs}" ]; then
+        # DEPLOY_DIR_DEB is shared by all multiconfigs with the same DISTRO and
+        # DISTRO_ARCH. It is transiently empty while do_dpkg_build of another
+        # multiconfig rebuilds this recipe, between sstate_clean() in its
+        # prefunc and sstate_install() in its postfunc. Skipping is safe: that
+        # multiconfig runs its own do_deploy_deb once the rebuild completed.
+        bbnote "${DEPLOY_DIR_DEB} is empty, leaving isar-apt untouched"
+        return
     fi
+    deb_clean
+    repo_add_packages "${REPO_ISAR_DIR}"/"${DISTRO}" \
+        "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" ${debs}
 }
 
 addtask deploy_deb after do_dpkg_build before do_build
-- 
2.55.0

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/isar-users/20260910140201.250854-4-felix.moessbauer%40siemens.com.

[PATCH 4/4] fix(dpkg-source): make sstate updates of DEPLOY_DIR_SRC atomic

['Felix Moessbauer' via isar-users]

DEPLOY_DIR_SRC is qualified by DISTRO and BPN, but not by DISTRO_ARCH or
MACHINE. All multiconfigs sharing DISTRO therefore run do_dpkg_source and
do_deploy_source of the very same recipe on one directory, concurrently
and without mutual exclusion.

do_dpkg_source updates DEPLOY_DIR_SRC via sstate. Its sstate_clean()
unlinks the .dsc and the tarballs before sstate_install() puts them back,
so a do_deploy_source running in parallel can observe DEPLOY_DIR_SRC empty
or partially populated. It then removes the source from isar-apt and
either adds nothing back, silently dropping it, or fails in reprepro
includedsc because the tarball referenced by the .dsc is not there yet.

Mirror the do_dpkg_build/do_deploy_deb fixes:

- Take a dedicated lock in the sstate clean/install of do_dpkg_source and
  in do_deploy_source via the sstate-lockfile task flag, so the latter
  never sees a partially populated directory. The lock lives next to, not
  inside, DEPLOY_DIR_SRC so that sstate_clean_manifest() cannot sweep it
  away.
- Scan DEPLOY_DIR_SRC first in do_deploy_source and skip the update when
  it is empty, leaving isar-apt untouched instead of dropping the source.
  That is safe: the multiconfig doing the rebuild runs its own
  do_deploy_source once do_dpkg_source completed.

Do not add the lock to do_clean: CLEANFUNCS also runs sstate_cleanall(),
which acquires the same lock, and flock() would deadlock on the nested
acquisition.

Fixes: 826acb32 ("dpkg: cache do_dpkg_source results in sstate")
Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
---
 meta/classes-recipe/dpkg-source.bbclass | 35 +++++++++++++++++++------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/meta/classes-recipe/dpkg-source.bbclass b/meta/classes-recipe/dpkg-source.bbclass
index 97cf9714..6f9091a8 100644
--- a/meta/classes-recipe/dpkg-source.bbclass
+++ b/meta/classes-recipe/dpkg-source.bbclass
@@ -15,10 +15,18 @@ DPKG_SOURCE_EXTRA_ARGS ?= "-I"
 DEBIAN_SOURCE ?= "${BPN}"
 SRCPKG_DIR = "${WORKDIR}/deploy-srcpkg"
 DEPLOY_DIR_SRC = "${DEPLOY_DIR}/isar-source/${DISTRO}/${BPN}"
+# DEPLOY_DIR_SRC is not qualified by DISTRO_ARCH or MACHINE, so all multiconfigs
+# sharing DISTRO run do_dpkg_source and do_deploy_source of the same recipe on
+# it, concurrently. Serialize the sstate clean/install of do_dpkg_source against
+# do_deploy_source, so that the latter never sees a partially populated
+# directory. Keep the lock next to, not inside, DEPLOY_DIR_SRC so that
+# sstate_clean_manifest() cannot sweep it away.
+DEPLOY_DIR_SRC_LOCK = "${DEPLOY_DIR}/isar-source/${DISTRO}/${BPN}.lock"
 
 do_dpkg_source[cleandirs] = "${SRCPKG_DIR}"
 do_dpkg_source[sstate-inputdirs] = "${SRCPKG_DIR}"
 do_dpkg_source[sstate-outputdirs] = "${DEPLOY_DIR_SRC}"
+do_dpkg_source[sstate-lockfile] = "${DEPLOY_DIR_SRC_LOCK}"
 do_dpkg_source() {
     # Create a .dsc file from source directory to use it with sbuild
     DEB_SOURCE_NAME=$(dpkg-parsechangelog --show-field Source --file ${WORKDIR}/${PPS}/debian/changelog)
@@ -42,24 +50,35 @@ addtask dpkg_source_setscene
 
 CLEANFUNCS += "deb_clean_source"
 
+# Do not guard this with DEPLOY_DIR_SRC_LOCK: it runs from CLEANFUNCS, which also
+# runs sstate_cleanall() taking that lock itself, and flock() would deadlock on
+# the nested acquisition.
 deb_clean_source() {
     repo_del_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
         "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" "${DEBIAN_SOURCE}"
 }
 
 do_deploy_source[depends] += "isar-apt:do_cache_config"
-do_deploy_source[lockfiles] = "${REPO_ISAR_DIR}/isar.lock"
+do_deploy_source[lockfiles] = "${REPO_ISAR_DIR}/isar.lock ${DEPLOY_DIR_SRC_LOCK}"
 do_deploy_source[dirs] = "${S} ${DEPLOY_DIR_SRC}"
 do_deploy_source() {
-    repo_del_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
-        "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" "${DEBIAN_SOURCE}"
+    # Scan DEPLOY_DIR_SRC first. An empty directory means another multiconfig is
+    # rebuilding this recipe: DEPLOY_DIR_SRC is transiently empty between
+    # sstate_clean() and sstate_install() of its do_dpkg_source. Removing the
+    # source from isar-apt and adding nothing back would drop it. Skipping is
+    # safe: that multiconfig runs its own do_deploy_source once the rebuild
+    # completed.
     DSC_FILE=$(find ${DEPLOY_DIR_SRC} -maxdepth 1 -name "${DEBIAN_SOURCE}_*.dsc")
-    if [ -n "${DSC_FILE}" ]; then
-        repo_add_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
-            "${REPO_ISAR_DB_DIR}"/"${DISTRO}" \
-            "${DEBDISTRONAME}" \
-            "${DSC_FILE}"
+    if [ -z "${DSC_FILE}" ]; then
+        bbnote "${DEPLOY_DIR_SRC} is empty, leaving isar-apt untouched"
+        return
     fi
+    repo_del_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
+        "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" "${DEBIAN_SOURCE}"
+    repo_add_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
+        "${REPO_ISAR_DB_DIR}"/"${DISTRO}" \
+        "${DEBDISTRONAME}" \
+        "${DSC_FILE}"
 }
 addtask deploy_source after do_dpkg_source
 
-- 
2.55.0

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/isar-users/20260910140201.250854-5-felix.moessbauer%40siemens.com.

Re: [PATCH 0/4] Fix various race conditions on multiconfig

[Zhihang Wei]

Applied to next, thanks.

Zhihang

On 9/10/26 16:01, 'Felix Moessbauer' via isar-users wrote:
> When running under multiconfig, various race conditions can occur due
> to simultaneous access to the isar-apt. We fix these by ensuring
> that the deployment of the source archive and the .debs is idempotent.
>
> While investigating this, we noticed that the existing deb_clean no
> longer removes the artifacts from the apt-cache, as it reads the "what
> to clean" from a dir that is previously removed by the internal
> clean logic of the sstate. This bug is fixed as well (p2).
>
> Note, that the root cause of the races has been found by an LLM,
> which had access to the build log of a failed execution.
>
> Best regards,
> Felix Moessbauer
> Siemens AG
>
> Felix Moessbauer (4):
>    fix(dpkg-build): make sstate updates of DEPLOY_DIR_DEB atomic
>    fix(dpkg-build): clean isar-apt by source package and arch
>    fix(dpkg-build): scan DEPLOY_DIR_DEB only once in do_deploy_deb
>    fix(dpkg-source): make sstate updates of DEPLOY_DIR_SRC atomic
>
>   meta/classes-recipe/dpkg-base.bbclass   | 53 +++++++++++++++++++------
>   meta/classes-recipe/dpkg-source.bbclass | 35 ++++++++++++----
>   meta/classes-recipe/repository.bbclass  | 22 ++++++++++
>   3 files changed, 89 insertions(+), 21 deletions(-)
>

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/isar-users/3fa39dd6-9046-4985-a1bb-65c5d75f56b9%40ilbers.de.