Horton

Thread

Commits

  1. Make pg_mkdir_p() tolerant of a concurrent directory creation.

  1. fix pg_mkdir_p to tolerate concurrent directory creation

    Andrew Dunstan <andrew@dunslane.net> — 2026-06-18T13:18:37Z

    While working on the pytest stuff, I found this issue when making it 
work on Windows, but the issue can exist everywhere. pg_mkdir_p can fail 
if there is a concurrent directory creation. This patch makes it 
tolerant of concurrent directory creation. To make it more robust it 
stops using stat() on Windows and instead uses GetFileAttributes().

I think this makes the function actually meet the contract set out in 
the file's header comment:

  * pg_mkdir_p --- create a directory and, if necessary, parent directories
  *
  * This is equivalent to "mkdir -p" except we don't complain if the target
  * directory already exists.

Thanks to Bryan Green for help with the Windows part.

There might be a few call sites that actually work around this bug that 
we can clean up.


cheers


andrew



--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  2. Re: fix pg_mkdir_p to tolerate concurrent directory creation

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-18T14:24:03Z

    Andrew Dunstan <andrew@dunslane.net> writes:
> While working on the pytest stuff, I found this issue when making it 
> work on Windows, but the issue can exist everywhere. pg_mkdir_p can fail 
> if there is a concurrent directory creation.

This bit:

+			if (errno != EEXIST || stat(path, &sb) != 0 || !S_ISDIR(sb.st_mode))
+			{
+				retval = -1;
+				break;
+			}

looks like it could corrupt the reported errno, ie stat() could
overwrite what mkdir() reported.

			regards, tom lane

  3. Re: fix pg_mkdir_p to tolerate concurrent directory creation

    Andrew Dunstan <andrew@dunslane.net> — 2026-06-18T15:37:49Z

    On 2026-06-18 Th 10:24 AM, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> While working on the pytest stuff, I found this issue when making it
>> work on Windows, but the issue can exist everywhere. pg_mkdir_p can fail
>> if there is a concurrent directory creation.
> This bit:
>
> +			if (errno != EEXIST || stat(path, &sb) != 0 || !S_ISDIR(sb.st_mode))
> +			{
> +				retval = -1;
> +				break;
> +			}
>
> looks like it could corrupt the reported errno, ie stat() could
> overwrite what mkdir() reported.
>
> 			


Oh, good catch, thanks! Here's a revised patch.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  4. Re: fix pg_mkdir_p to tolerate concurrent directory creation

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-18T15:43:38Z

    Andrew Dunstan <andrew@dunslane.net> writes:
> Oh, good catch, thanks! Here's a revised patch.

The Unix side of this LGTM.  I have no idea about the Windows
half (... but are we sure GetFileAttributes() can't change errno?)

			regards, tom lane

  5. Re: fix pg_mkdir_p to tolerate concurrent directory creation

    Andrew Dunstan <andrew@dunslane.net> — 2026-06-18T16:56:25Z

    On 2026-06-18 Th 11:43 AM, Tom Lane wrote:
> Andrew Dunstan<andrew@dunslane.net> writes:
>> Oh, good catch, thanks! Here's a revised patch.
> The Unix side of this LGTM.  I have no idea about the Windows
> half (... but are we sure GetFileAttributes() can't change errno?)
>
> 			


I'm told it does not. Google says:


GetFileAttributes does not set errno. Because it is a Win32 API function 
rather than a standard C library function, it sets the thread-local 
Last-Error Code instead.


|
|

|cheers|

|
|

|andrew|


--
Andrew Dunstan
EDB:https://www.enterprisedb.com

  6. Re: fix pg_mkdir_p to tolerate concurrent directory creation

    Álvaro Herrera <alvherre@kurilemu.de> — 2026-06-19T14:01:47Z

    On 2026-Jun-18, Andrew Dunstan wrote:

> 
> While working on the pytest stuff, I found this issue when making it work on
> Windows, but the issue can exist everywhere. pg_mkdir_p can fail if there is
> a concurrent directory creation. This patch makes it tolerant of concurrent
> directory creation.

Hmm, should the 'break's in the loop be 'continue's?  If you're creating
path a/b/c, and a concurrent process beats you but is only attempting to
do a/b, then with the break you'll fail to create the final c.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
Bob [Floyd] used to say that he was planning to get a Ph.D. by the "green
stamp method," namely by saving envelopes addressed to him as 'Dr. Floyd'.
After collecting 500 such letters, he mused, a university somewhere in
Arizona would probably grant him a degree.              (Don Knuth)

  7. Re: fix pg_mkdir_p to tolerate concurrent directory creation

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-19T14:51:02Z

    =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:
> Hmm, should the 'break's in the loop be 'continue's?  If you're creating
> path a/b/c, and a concurrent process beats you but is only attempting to
> do a/b, then with the break you'll fail to create the final c.

Maybe I'm confused, but I think it's right as-is.  The "break"s are
places where we're giving up and reporting a hard error.  If we want
to continue to the next directory level, we just fall through ---
and doing that with "continue" would be wrong because it'd miss the
step at the bottom of the loop:

		if (!last)
			*p = '/';


			regards, tom lane

  8. Re: fix pg_mkdir_p to tolerate concurrent directory creation

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-19T15:07:58Z

    Actually ... given this change, why don't we drop the initial stat()
call?  Just try mkdir(), and move on if it succeeds.  If not, but
there's already a directory there, we're good.  That eliminates the
race condition without duplicating code.

			regards, tom lane

  9. Re: fix pg_mkdir_p to tolerate concurrent directory creation

    Andrew Dunstan <andrew@dunslane.net> — 2026-06-19T15:40:52Z

    On 2026-06-19 Fr 11:07 AM, Tom Lane wrote:
> Actually ... given this change, why don't we drop the initial stat()
> call?  Just try mkdir(), and move on if it succeeds.  If not, but
> there's already a directory there, we're good.  That eliminates the
> race condition without duplicating code.


LGTM, thanks!


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  10. Re: fix pg_mkdir_p to tolerate concurrent directory creation

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-19T16:52:37Z

    Andrew Dunstan <andrew@dunslane.net> writes:
> LGTM, thanks!

OK, pushed.

			regards, tom lane

  11. Re: fix pg_mkdir_p to tolerate concurrent directory creation

    Andrew Dunstan <andrew@dunslane.net> — 2026-06-19T18:39:09Z

    On 2026-06-19 Fr 12:52 PM, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> LGTM, thanks!
> OK, pushed.
>
> 			


Thanks!


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com