diff mbox series

[2/5] bitbake-setup: ensure paths with timestamps in them are unique

Message ID 20260109132000.2372791-2-alex.kanavin@gmail.com
State Accepted, archived
Commit e96b97e176b2fcc0bfdcf0151dff6df3ecaefcb3
Headers show
Series [1/5] doc: document fixed revisions override in bitbake-setup manual | expand

Commit Message

Alexander Kanavin Jan. 9, 2026, 1:19 p.m. UTC
From: Alexander Kanavin <alex@linutronix.de>

There have been instances where directories with timestamps
in them are created within the same second, and their paths
clash as the timestamp in them is the same for both, as the
timestamp is accurate to 1 second.

This adds a check for the directory existence and then adds
an ever-increasing counter until there's a path that isn't yet
created.

[YOCTO #16121]

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 bin/bitbake-setup | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Comments

Quentin Schulz Jan. 15, 2026, 12:03 p.m. UTC | #1
Hi Alex,

On 1/9/26 2:19 PM, Alexander Kanavin via lists.openembedded.org wrote:
> From: Alexander Kanavin <alex@linutronix.de>
> 
> There have been instances where directories with timestamps
> in them are created within the same second, and their paths
> clash as the timestamp in them is the same for both, as the
> timestamp is accurate to 1 second.
> 
> This adds a check for the directory existence and then adds
> an ever-increasing counter until there's a path that isn't yet
> created.
> 

That's just making the race window smaller but it still exists.

> [YOCTO #16121]
> 
> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> ---
>   bin/bitbake-setup | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/bin/bitbake-setup b/bin/bitbake-setup
> index 06255b112..4ccf47e87 100755
> --- a/bin/bitbake-setup
> +++ b/bin/bitbake-setup
> @@ -93,6 +93,17 @@ def _write_layer_list(dest, repodirs):
>       with open(layers_f, 'w') as f:
>           json.dump({"version":"1.0","layers":layers}, f, sort_keys=True, indent=4)
>   
> +def add_unique_timestamp_to_path(path):
> +    timestamp = time.strftime("%Y%m%d%H%M%S")
> +    path_unique = "{}.{}".format(path, timestamp)
> +    if os.path.exists(path_unique):
> +        import itertools
> +        for i in itertools.count(start=1):
> +            path_unique = "{}.{}.{}".format(path, timestamp, i)
> +            if not os.path.exists(path_unique):
> +                break
> +    return path_unique
> +

Let's say you have two instances of bitbake-setup running at the same 
time and entering this function.

They may both very well exit with the same path_unique as you only check 
for the presence of the directory and not create it, so they both will 
find .10 doesn't exist yet and decide to have this suffix.

The logic should be something like (untested) instead:

now = time.strftime("%Y%m%d%H%M%S")
for i in itertools.count(start=0):
     suffix = '' if i == 0 else f'.{i}'
     newpath = f'{path}.{now}{suffix}'
     try:
         os.mkdir(f'{newpath}')
     except FileExistsError:
         continue
     else:
         return newpath

Cheers,
Quentin
Alexander Kanavin Jan. 15, 2026, 12:24 p.m. UTC | #2
On Thu, 15 Jan 2026 at 13:03, Quentin Schulz <quentin.schulz@cherry.de> wrote:
> Let's say you have two instances of bitbake-setup running at the same
> time and entering this function.

This patch is not fixing a race between parallel invocations. It fixes
two *consecutive* invocations of bitbake-setup that happen within the
same second. The commit message should be clearer, yes, but it's also
already merged.

Parallel invocations of bitbake-setup that handle the same setup would
step on each other in many different ways otherwise, and it's simply
not supported at all.

Alex
diff mbox series

Patch

diff --git a/bin/bitbake-setup b/bin/bitbake-setup
index 06255b112..4ccf47e87 100755
--- a/bin/bitbake-setup
+++ b/bin/bitbake-setup
@@ -93,6 +93,17 @@  def _write_layer_list(dest, repodirs):
     with open(layers_f, 'w') as f:
         json.dump({"version":"1.0","layers":layers}, f, sort_keys=True, indent=4)
 
+def add_unique_timestamp_to_path(path):
+    timestamp = time.strftime("%Y%m%d%H%M%S")
+    path_unique = "{}.{}".format(path, timestamp)
+    if os.path.exists(path_unique):
+        import itertools
+        for i in itertools.count(start=1):
+            path_unique = "{}.{}.{}".format(path, timestamp, i)
+            if not os.path.exists(path_unique):
+                break
+    return path_unique
+
 def checkout_layers(layers, layerdir, d):
     def _checkout_git_remote(r_remote, repodir, layers_fixed_revisions):
         rev = r_remote['rev']
@@ -242,9 +253,8 @@  def setup_bitbake_build(bitbake_config, layerdir, setupdir, thisdir, update_bb_c
         raise Exception("Cannot complete setting up a bitbake build directory from OpenEmbedded template '{}' as oe-setup-build was not found in any layers; please use oe-init-build-env manually.".format(template))
 
     bitbake_confdir = os.path.join(bitbake_builddir, 'conf')
-    timestamp = time.strftime("%Y%m%d%H%M%S")
-    backup_bitbake_confdir = os.path.join(bitbake_builddir, 'conf-backup.{}'.format(timestamp))
-    upstream_bitbake_confdir = os.path.join(bitbake_builddir, 'conf-upstream.{}'.format(timestamp))
+    backup_bitbake_confdir = add_unique_timestamp_to_path(os.path.join(bitbake_builddir, 'conf-backup'))
+    upstream_bitbake_confdir = add_unique_timestamp_to_path(os.path.join(bitbake_builddir, 'conf-upstream'))
 
     if os.path.exists(bitbake_confdir):
         os.rename(bitbake_confdir, backup_bitbake_confdir)
@@ -760,7 +770,7 @@  def install_buildtools(top_dir, settings, args, d):
         shutil.rmtree(buildtools_install_dir)
 
     install_buildtools = os.path.join(args.setup_dir, 'layers/oe-scripts/install-buildtools')
-    buildtools_download_dir = os.path.join(args.setup_dir, 'buildtools-downloads/{}'.format(time.strftime("%Y%m%d%H%M%S")))
+    buildtools_download_dir = add_unique_timestamp_to_path(os.path.join(args.setup_dir, 'buildtools-downloads/buildtools'))
     logger.plain("Buildtools archive is downloaded into {} and its content installed into {}".format(buildtools_download_dir, buildtools_install_dir))
     subprocess.check_call("{} -d {} --downloads-directory {}".format(install_buildtools, buildtools_install_dir, buildtools_download_dir), shell=True)