Skip to content

Prevent race conditions when downloading NodeJS#424

Open
GlassOfWhiskey wants to merge 1 commit into
mainfrom
singularity-nodejs-race-conditions
Open

Prevent race conditions when downloading NodeJS#424
GlassOfWhiskey wants to merge 1 commit into
mainfrom
singularity-nodejs-race-conditions

Conversation

@GlassOfWhiskey
Copy link
Copy Markdown
Collaborator

Singularity downloads the NodeJS image locally (either in the current folder or in the CWL_SINGULARITY_CACHE path. If multiple processes download the same image concurrently, the second one fails because the image already exists. This PR prevents this behaviour by letting the execution proceed when an image is already there.

Singularity downloads the NodeJS image locally (either in the
current folder or in the `CWL_SINGULARITY_CACHE` path. If multiple
processes download the same image concurrently, the second one
fails because the image already exists. This PR prevents this
behaviour by letting the execution proceed when an image is already
there.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

❌ Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.77%. Comparing base (d206288) to head (d282920).

Files with missing lines Patch % Lines
src/cwl_utils/sandboxjs.py 57.14% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #424      +/-   ##
==========================================
- Coverage   37.77%   37.77%   -0.01%     
==========================================
  Files          50       50              
  Lines       36760    36764       +4     
  Branches     9531     9532       +1     
==========================================
+ Hits        13886    13887       +1     
- Misses      19941    19944       +3     
  Partials     2933     2933              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@GlassOfWhiskey
Copy link
Copy Markdown
Collaborator Author

@kinow this should fix the race condition you encountered with NodeJS and Singularity

@kinow
Copy link
Copy Markdown
Member

kinow commented May 18, 2026

Will try to test it today, @GlassOfWhiskey . Thank you!

Copy link
Copy Markdown
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used the PR in StreamFlow, alpha-unito/streamflow#1054:

(streamflow) [curso348@login209-18 streamflow]$ git log -n 1
commit bec7ab63f5d13f389e2da71f4ba54aa76611ccbf (HEAD -> node-with-singularity, upstream/node-with-singularity)
Author: GlassOfWhiskey <iacopo.c92@gmail.com>
Date:   Tue May 5 00:00:40 2026 +0200

    Fix NodeJS retrieval when Docker is unavailable
    
    Fix #1053 by adding `_get_container_engine()` to detect the available
    container runtime by probing `docker`, `singularity`, `podman`, and
    `udocker` in order, caching the result with `@cached(FIFOCache(1))`
    from `cachebox`. The detected engine is passed as `container_engine`
    to `cwl_utils.expression.interpolate()` in `eval_expression()`,
    fixing CWL JavaScript expression evaluation on systems without Docker.

Then cloned this repository, and checked out this branch.

commit d28292033704f7eac76007ffa45f1bffd74a829e (HEAD -> singularity-nodejs-race-conditions, upstream/singularity-nodejs-race-conditions)
Author: GlassOfWhiskey <iacopo.c92@gmail.com>
Date:   Mon May 18 11:21:46 2026 +0200

    Prevent race conditions when downloading NodeJS
    
    Singularity downloads the NodeJS image locally (either in the
    current folder or in the `CWL_SINGULARITY_CACHE` path. If multiple
    processes download the same image concurrently, the second one
    fails because the image already exists. This PR prevents this
    behaviour by letting the execution proceed when an image is already
    there.

And installed it,

Building wheels for collected packages: cwl-utils
  Building editable for cwl-utils (pyproject.toml) ... done
  Created wheel for cwl-utils: filename=cwl_utils-0.41-py3-none-any.whl size=8292 sha256=1be7ae4fdd6cd660236197be97bbc64d6c5bb50eeb1406dccf7372d93437fe67
  Stored in directory: /tmp/pip-ephem-wheel-cache-tr085one/wheels/5f/bb/dd/2f3af69a948fdf1c19aa2b6ae3fe6bd2ea8e2f1572bba84763
Successfully built cwl-utils
Installing collected packages: cwl-utils
  Attempting uninstall: cwl-utils
    Found existing installation: cwl-utils 0.41
    Uninstalling cwl-utils-0.41:
      Successfully uninstalled cwl-utils-0.41
Successfully installed cwl-utils-0.41

Entered the realpath with cd $(pwd -P) (due to the bug with symlinks, common-workflow-language/cwltest#281), and tried to run it.

======================================================================================================================= 26 failed, 170 passed, 1 skipped in 31.23s =======================================================================================================================

@GlassOfWhiskey maybe we can troubleshoot it later. Maybe I'm doing wrong as I was launching this while waiting for other jobs to complete in Slurm. I will try again.

@GlassOfWhiskey
Copy link
Copy Markdown
Collaborator Author

But you know why did they fail? Without logs it is difficult to understand if fails are related to the PR or not :(

@kinow
Copy link
Copy Markdown
Member

kinow commented May 19, 2026

They failed in the even loop of streamflow. I tried to run a workflow directly and it crashed with the same error. I tried installing node, but the same happened. I suspect it is more the lack of sleep, which is why I didn' want to bother you with the logs 😬 Let me try from scratch installing everything again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants