[PATCH] Windows: Fix set_unbuffered_mode.o file rename race

[PATCH] Windows: Fix set_unbuffered_mode.o file rename race

[Pedro Alves]

The atomic file rename for set_unbuffered_mode.o can fail in this scenario:

 | process A                       | process B                 |
 |---------------------------------+---------------------------|
 | compiles temp .o                | compiles temp .o          |
 | moves .o                        |                           |
 | links with .o file (locks file) | moves .o (fails w/ EBUSY) |

Here's what it looks like:

  builtin_spawn -ignore SIGHUP /mingw64/bin/clang -fdiagnostics-color=never -Wno-unknown-warning-option -w -c -o /c/msys2/home/alves/gdb/build-testsuite/temp/53930/set_unbuffered_mode-c.o /c/rocgdb/src/gdb/testsuite/lib/set_unbuffered_mode.c
  pid is 54259 -54259
  pid is -1
  output is  status 0
  ERROR: tcl error sourcing /c/rocgdb/src/gdb/testsuite/gdb.base/step-over-no-symbols.exp.
  ERROR: tcl error code POSIX EBUSY {file busy}
  ERROR: error renaming "/c/msys2/home/alves/gdb/build-testsuite/temp/53930/set_unbuffered_mode.o" to "/c/msys2/home/alves/gdb/build-testsuite/set_unbuffered_mode.o": file busy
      while executing
  "file rename -force --  $unbuf_obj  $gdb_saved_set_unbuffered_mode_obj"
      (procedure "gdb_compile" line 559)
      invoked from within
  "gdb_compile $source $dest $type $options"
      (procedure "gdb_compile" line 42)
      invoked from within
  "$func $objects "${binfile}" executable $options"
      (procedure "build_executable_from_specs" line 50)
      invoked from within
  "build_executable_from_specs {*}$arglist"
      (procedure "build_executable" line 11)
      invoked from within
  "build_executable "failed to build" ${testfile} $srcfile"
      (file "/c/rocgdb/src/gdb/testsuite/gdb.base/step-over-no-symbols.exp" line 21)
      invoked from within
  "source /c/rocgdb/src/gdb/testsuite/gdb.base/step-over-no-symbols.exp"
      ("uplevel" body line 1)
      invoked from within
  "uplevel #0 source /c/rocgdb/src/gdb/testsuite/gdb.base/step-over-no-symbols.exp"
      invoked from within
  "catch "uplevel #0 source $test_file_name" msg"
  UNRESOLVED: gdb.base/step-over-no-symbols.exp: testcase '/c/rocgdb/src/gdb/testsuite/gdb.base/step-over-no-symbols.exp' aborted due to Tcl error

If we get EBUSY, it's because another parallel worker already managed
to build and move its set_unbuffered_mode.o copy to the final
destination.  So fix it by simply ignoring EBUSY.  Put the rename in
its own procedure, as I expect this will be used in more places.

(Note: the set_unbuffered_mode.o path is Windows-specific.)

Change-Id: I6a32d17364a19337d7f55e4376de736e1cca799d
---
 gdb/testsuite/lib/gdb.exp | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index a40c87c6727..db5a0dd7afc 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -6402,6 +6402,25 @@ proc quote_for_host { args } {
     return $str
 }
 
+# Rename SRC to DST, ignoring EBUSY.  This is used when multiple
+# parallel workers all want to rename their copy of SRC to DST, as an
+# atomic commit, and it doesn't matter which one wins, as all the
+# copies are identical.
+proc file_rename_atomic {src dst} {
+    set rc [catch { file rename -force -- $src $dst } err opts]
+
+    if {$rc} {
+	set code [dict get $opts -errorcode]
+	if {[llength $code] >= 2 && [lindex $code 1] eq "EBUSY"} {
+	    # Normal parallel race loss.
+	} else {
+	    error $err $opts
+	}
+    }
+
+    return $rc
+}
+
 # Compile source files specified by SOURCE into a binary of type TYPE at path
 # DEST.  gdb_compile is implemented using DejaGnu's target_compile, so the type
 # parameter and most options are passed directly to it.
@@ -6922,7 +6941,7 @@ proc gdb_compile {source dest type options} {
 		    # Make sure to write the .o file atomically.
 		    # (Note GDB_PARALLEL mode does not support remote
 		    # host testing.)
-		    file rename -force -- $unbuf_obj $gdb_saved_set_unbuffered_mode_obj
+		    file_rename_atomic $unbuf_obj $gdb_saved_set_unbuffered_mode_obj
 		} else {
 		    remote_download host $unbuf_obj $gdb_saved_set_unbuffered_mode_obj
 		}

base-commit: b43c744924b3f5df3677dbe97d12608e63ca5a4d
-- 
2.54.0

Re: [PATCH] Windows: Fix set_unbuffered_mode.o file rename race

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> If we get EBUSY, it's because another parallel worker already managed
Pedro> to build and move its set_unbuffered_mode.o copy to the final
Pedro> destination.  So fix it by simply ignoring EBUSY.  Put the rename in
Pedro> its own procedure, as I expect this will be used in more places.

This looks ok to me, thanks.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH] Windows: Fix set_unbuffered_mode.o file rename race

[Pedro Alves]

On 2026-07-21 17:43, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> If we get EBUSY, it's because another parallel worker already managed
> Pedro> to build and move its set_unbuffered_mode.o copy to the final
> Pedro> destination.  So fix it by simply ignoring EBUSY.  Put the rename in
> Pedro> its own procedure, as I expect this will be used in more places.
> 
> This looks ok to me, thanks.
> Approved-By: Tom Tromey <tom@tromey.com>

Thank you.  Meanwhile, the manifest patch went in, which adds another path doing
the exact same, which needs the same fix.  I did the obvious tweak to the patch
to call the new proc from two places, and merged it, as below.

Thanks again,
Pedro Alves

From 81a1c8f753e506b3890db4b1254df05aa67a0230 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Fri, 28 Nov 2025 11:28:06 +0000
Subject: [PATCH] Windows: Fix set_unbuffered_mode.o file rename race

The atomic file rename for set_unbuffered_mode.o can fail in this scenario:

 | process A                       | process B                 |
 |---------------------------------+---------------------------|
 | compiles temp .o                | compiles temp .o          |
 | moves .o                        |                           |
 | links with .o file (locks file) | moves .o (fails w/ EBUSY) |

Here's what it looks like:

  builtin_spawn -ignore SIGHUP /mingw64/bin/clang -fdiagnostics-color=never -Wno-unknown-warning-option -w -c -o /c/msys2/home/alves/gdb/build-testsuite/temp/53930/set_unbuffered_mode-c.o /c/rocgdb/src/gdb/testsuite/lib/set_unbuffered_mode.c
  pid is 54259 -54259
  pid is -1
  output is  status 0
  ERROR: tcl error sourcing /c/rocgdb/src/gdb/testsuite/gdb.base/step-over-no-symbols.exp.
  ERROR: tcl error code POSIX EBUSY {file busy}
  ERROR: error renaming "/c/msys2/home/alves/gdb/build-testsuite/temp/53930/set_unbuffered_mode.o" to "/c/msys2/home/alves/gdb/build-testsuite/set_unbuffered_mode.o": file busy
      while executing
  "file rename -force --  $unbuf_obj  $gdb_saved_set_unbuffered_mode_obj"
      (procedure "gdb_compile" line 559)
      invoked from within
  "gdb_compile $source $dest $type $options"
      (procedure "gdb_compile" line 42)
      invoked from within
  "$func $objects "${binfile}" executable $options"
      (procedure "build_executable_from_specs" line 50)
      invoked from within
  "build_executable_from_specs {*}$arglist"
      (procedure "build_executable" line 11)
      invoked from within
  "build_executable "failed to build" ${testfile} $srcfile"
      (file "/c/rocgdb/src/gdb/testsuite/gdb.base/step-over-no-symbols.exp" line 21)
      invoked from within
  "source /c/rocgdb/src/gdb/testsuite/gdb.base/step-over-no-symbols.exp"
      ("uplevel" body line 1)
      invoked from within
  "uplevel #0 source /c/rocgdb/src/gdb/testsuite/gdb.base/step-over-no-symbols.exp"
      invoked from within
  "catch "uplevel #0 source $test_file_name" msg"
  UNRESOLVED: gdb.base/step-over-no-symbols.exp: testcase '/c/rocgdb/src/gdb/testsuite/gdb.base/step-over-no-symbols.exp' aborted due to Tcl error

If we get EBUSY, it's because another parallel worker already managed
to build and move its set_unbuffered_mode.o copy to the final
destination.  So fix it by simply ignoring EBUSY.

gdb_windows_manifest_obj has similar code with the same problem, so
put the atomic rename in a new file_rename_atomic procedure, and use
it from both places.

(Note: both the set_unbuffered_mode.o path and
gdb_windows_manifest_obj are Windows-specific.)

Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I6a32d17364a19337d7f55e4376de736e1cca799d
---
 gdb/testsuite/lib/gdb.exp | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 9b86d53be08..41e9a8c721e 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -6408,6 +6408,25 @@ proc quote_for_host { args } {
     return $str
 }
 
+# Rename SRC to DST, ignoring EBUSY.  This is used when multiple
+# parallel workers all want to rename their copy of SRC to DST, as an
+# atomic commit, and it doesn't matter which one wins, as all the
+# copies are identical.
+proc file_rename_atomic {src dst} {
+    set rc [catch { file rename -force -- $src $dst } err opts]
+
+    if {$rc} {
+	set code [dict get $opts -errorcode]
+	if {[llength $code] >= 2 && [lindex $code 1] eq "EBUSY"} {
+	    # Normal parallel race loss.
+	} else {
+	    error $err $opts
+	}
+    }
+
+    return $rc
+}
+
 # Set while linker_supports_manifest_embed is running its test link,
 # so that the inner gdb_compile that link goes through skips the
 # manifest-embedding logic and doesn't recurse back into the probe.
@@ -6491,7 +6510,7 @@ proc gdb_windows_manifest_obj {} {
     if {[info exists ::GDB_PARALLEL]} {
 	# Make sure to write the .o file atomically.  (Note
 	# GDB_PARALLEL mode does not support remote host testing.)
-	file rename -force -- $obj $saved
+	file_rename_atomic $obj $saved
     } else {
 	remote_download host $obj $saved
     }
@@ -7019,7 +7038,7 @@ proc gdb_compile {source dest type options} {
 		    # Make sure to write the .o file atomically.
 		    # (Note GDB_PARALLEL mode does not support remote
 		    # host testing.)
-		    file rename -force -- $unbuf_obj $gdb_saved_set_unbuffered_mode_obj
+		    file_rename_atomic $unbuf_obj $gdb_saved_set_unbuffered_mode_obj
 		} else {
 		    remote_download host $unbuf_obj $gdb_saved_set_unbuffered_mode_obj
 		}

base-commit: 33f0797fff4dcac33092bfde19105c1eeb483526
-- 
2.54.0

[PATCH] gdb/testsuite: Use file_rename_atomic in gdb_do_cache too

[Pedro Alves]

On 2026-07-22 16:07, Pedro Alves wrote:
> On 2026-07-21 17:43, Tom Tromey wrote:
>>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
>>
>> Pedro> If we get EBUSY, it's because another parallel worker already managed
>> Pedro> to build and move its set_unbuffered_mode.o copy to the final
>> Pedro> destination.  So fix it by simply ignoring EBUSY.  Put the rename in
>> Pedro> its own procedure, as I expect this will be used in more places.
>>
>> This looks ok to me, thanks.
>> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Thank you.  Meanwhile, the manifest patch went in, which adds another path doing
> the exact same, which needs the same fix.  I did the obvious tweak to the patch
> to call the new proc from two places, and merged it, as below.

After merging, I recalled that Claudiu mentioned that he was seeing EBUSY errors
in the caching code too.  For some reason, probably just "lucky" timing, I don't recall
seeing it trigger on my machine, but looking at the code, the pattern is clear, and the
fix becomes pretty obvious, I think -- just another spot that should use file_rename_atomic.

I grepped for "file rename" and didn't see any other spot that might need this.

From 1b1e6c373467a889bc91499cf49fa874bd92a2bc Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 22 Jul 2026 16:08:55 +0100
Subject: [PATCH] gdb/testsuite: Use file_rename_atomic in gdb_do_cache too

An earlier commit ("Windows: Fix set_unbuffered_mode.o file rename
race") introduced file_rename_atomic to ignore EBUSY when multiple
parallel workers race to rename their identical copy of a file to a
shared final destination, and converted the two atomic renames in
gdb.exp to use it.

gdb_do_cache in cache.exp does the same thing: in GDB_PARALLEL mode,
each worker writes the results cache to a per-pid temporary file and
then atomically renames it into place, so it can hit the same EBUSY
race on Windows.  It was missed by that commit.

Fix it by using file_rename_atomic there too.

Change-Id: I9780ed4989f9c4e9daf7143280cd63a65c6918ed
---
 gdb/testsuite/lib/cache.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/cache.exp b/gdb/testsuite/lib/cache.exp
index 1ba8c881716..1e2d470773a 100644
--- a/gdb/testsuite/lib/cache.exp
+++ b/gdb/testsuite/lib/cache.exp
@@ -262,7 +262,7 @@ proc gdb_do_cache {name args} {
 	puts $fd $gdb_data_cache(${cache_name},exit)
 	puts $fd $gdb_data_cache(${cache_name},also_called)
 	close $fd
-	file rename -force -- $cache_filename.[pid] $cache_filename
+	file_rename_atomic $cache_filename.[pid] $cache_filename
     }
     gdb_cache_maybe_gdb_exit $name $gdb_data_cache(${cache_name},exit) \
 	$gdb_data_cache(${cache_name},also_called)

base-commit: 81a1c8f753e506b3890db4b1254df05aa67a0230
-- 
2.54.0

Re: [PATCH] gdb/testsuite: Use file_rename_atomic in gdb_do_cache too

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> gdb_do_cache in cache.exp does the same thing: in GDB_PARALLEL mode,
Pedro> each worker writes the results cache to a per-pid temporary file and
Pedro> then atomically renames it into place, so it can hit the same EBUSY
Pedro> race on Windows.  It was missed by that commit.

Pedro> Fix it by using file_rename_atomic there too.

Looks good.
Approved-By: Tom Tromey <tom@tromey.com>

Tom