Revert "chore(cleanup): lxc-helpers does not need a global lock (#1047)" #1103

Merged

earl-warren merged 1 commit from earl-warren/runner:wip-lock into main

2025-10-20 21:05:06 +00:00

earl-warren commented

2025-10-20 14:59:04 +00:00

Contributor

This reverts commit 996ac343ee.

The lock is still needed to guard against the following scenario.

 function build_template_act() {
     local name="$(template_act)"
     if lxc_exists_and_apt_not_old $name ; then
       return 0
     fi
     lxc_build_template $(lxc_template_release) $name
     lxc_container_start $name
     install_nodejs $name
     lxc_container_stop $name
 }

two or more jobs start
one of them creates the act template (lxc_build_template $(lxc_template_release) $name) - lxc-helpers now has transactions and they won't race against each other
once it is built all jobs will then try to install node in the container that was just built and race against each other

with a global lock only the first one will build and populate the act template. The other will then do nothing because it already exists.

The bug can trivially be reproduced with:

on:
  pull_request:

jobs:
  test1:
    runs-on: lxc
    steps:
      - run: echo OK1
  test2:
    runs-on: lxc
    steps:
      - run: echo OK2
  test3:
    runs-on: lxc
    steps:
      - run: echo OK3

other
- PR: Revert "chore(cleanup): lxc-helpers does not need a global lock (#1047)"

This reverts commit 996ac343ee155a4b47d89a6bf1854feb1c04fdb2. The lock is still needed to guard against the following scenario. https://code.forgejo.org/forgejo/runner/src/commit/d92a892ecebb7d5d362f252c487635c1fb9a4c6e/act/runner/run_context.go#L225-L236 - two or more jobs start - one of them creates the act template (`lxc_build_template $(lxc_template_release) $name`) - lxc-helpers now has transactions and they won't race against each other - once it is built all jobs will then try to install node in the container that was just built and race against each other with a global lock only the first one will build and populate the act template. The other will then do nothing because it already exists. The bug can trivially be reproduced with: ```yaml on: pull_request: jobs: test1: runs-on: lxc steps: - run: echo OK1 test2: runs-on: lxc steps: - run: echo OK2 test3: runs-on: lxc steps: - run: echo OK3 ```   - other - [PR](https://code.forgejo.org/forgejo/runner/pulls/1103): Revert "chore(cleanup): lxc-helpers does not need a global lock (#1047)"

earl-warren added the

Kind/Bug

label

2025-10-20 14:59:04 +00:00

earl-warren self-assigned this

2025-10-20 14:59:04 +00:00

earl-warren requested reviews from viceice, limiting-factor

2025-10-20 14:59:14 +00:00

earl-warren force-pushed wip-lock from 6a24559a4a

cascade / forgejo (pull_request_target) Has been skipped

Details

cascade / debug (pull_request_target) Has been skipped