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

[Pedro Alves <pedro@palves.net>]

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