MDEV-38494 .mariadb_history rename race condition by MooSayed1 · Pull Request #4699 · MariaDB/server

MooSayed1 · 2026-02-25T19:28:26Z

MDEV-38494: `.mariadb_history` rename race condition

The Jira issue number for this PR is: MDEV-38494

Description

When multiple mariadb CLI sessions exit at the same time (e.g. multiple
terminal tabs), all sessions write their history to the same temp file path
~/.mariadb_history.TMP and then rename it. The second rename fails because
the first session already moved the file:

bin/mariadb: Error on rename of '~/.mariadb_history.TMP' to
             '~/.mariadb_history' (Errcode: 2 "No such file or directory")

The root cause is in client/mysql.cc. The temp filename is constructed as:

sprintf(histfile_tmp, "%s.TMP", histfile);

All concurrent sessions use the same .TMP path. When session A renames
it to the final history file, session B's subsequent rename fails with ENOENT
because the file is already gone.

The fix includes the process PID in the temp filename so each session has its
own unique rename target:

// Include PID in temp file name to prevent concurrent-session rename races.
sprintf(histfile_tmp, "%s.TMP%lu", histfile, (unsigned long) getpid());

Release Notes

mariadb CLI no longer shows Error on rename of '.mariadb_history.TMP'
when multiple sessions exit concurrently.

How can this PR be tested?

./mtr main.mariadb_history_race

The test pre-creates $HISTFILE.TMP as a sentinel, then runs an interactive
mariadb session via a Python PTY wrapper (so isatty() returns true and
history is actually written). With the fix, the client uses $HISTFILE.TMP<PID>
and the sentinel survives. Without the fix, the client overwrites and renames
the sentinel → test fails.

Basing the PR against the correct MariaDB version

This is a bug fix. The earliest affected branch is 10.11, so this PR targets
upstream/10.11.

PR quality check

I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

MooSayed1 · 2026-02-25T19:29:40Z

I used a Python helper script (mariadb_history_race.py) alongside the .test file to create a PTY session.
so using a .py helper file acceptable in the main directory?

gkodinov · 2026-02-26T08:37:00Z

I used a Python helper script (mariadb_history_race.py) alongside the .test file to create a PTY session. so using a .py helper file acceptable in the main directory?

we usually use perl. mtr can run perl scripts directly

svoj · 2026-02-26T08:52:20Z

There's mysql-test/main/mysql-interactive.test that tests CLI in interactive mode.

The fix does silence error message, but doesn't solve loss of history records from concurrent sessions. OTOH bug report didn't request it.

gkodinov

Thank you for your contribution. This is a preliminary review.

gkodinov · 2026-02-26T08:40:52Z

      read_history(histfile);
      if (!(histfile_tmp= (char*) my_malloc(PSI_NOT_INSTRUMENTED,
-                                            strlen(histfile) + 5, MYF(MY_WME))))
+                                            strlen(histfile) + 20, MYF(MY_WME))))


I'd add a comment stating why it's 20. Ideally I'd use some sizeof()

gkodinov · 2026-02-26T08:46:44Z

      }
-      sprintf(histfile_tmp, "%s.TMP", histfile);
+      // Include PID in temp file name to prevent concurrent-session rename races.
+      sprintf(histfile_tmp, "%s.TMP%lu", histfile, (unsigned long) getpid());


please change to snprintf().

Also, I'd rather prefer %s.%lu.TMP myself. Extensions are how the OS recognizes file types. Also, pid_t is a signed type: https://man.archlinux.org/man/pid_t.3type.en. I wonder what does this do to your naming scheme. I'd take the absolute value.

gkodinov · 2026-02-26T08:47:42Z

@@ -0,0 +1,112 @@
+#!/usr/bin/env python3


please use perl. This is what mtr uses and understands. I'd personally avoid the whole thing and make sure mysql's --skip-batch works as one would expect and sets batch to 0.

MooSayed1 · 2026-02-26T21:50:04Z

Thanks for helping and reviewing @svoj @gkodinov
now I'm working on removing socat from the test entirely. Instead of simulating a PTY, implementing --skip-batch support — changing the --batch option from GET_NO_ARG to GET_BOOL so that MariaDB's built-in --skip-xxx negation works.
so just confirm that approtch looks okay to u if so i'll continue with it.

gkodinov · 2026-02-27T10:44:16Z

Thanks for helping and reviewing @svoj @gkodinov now I'm working on removing socat from the test entirely. Instead of simulating a PTY, implementing --skip-batch support — changing the --batch option from GET_NO_ARG to GET_BOOL so that MariaDB's built-in --skip-xxx negation works. so just confirm that approtch looks okay to u if so i'll continue with it.

It's probably best done in a separate PR I guess. Since, as a change in behavior, it needs to go to main.
But it does sound right to me.

gkodinov

LGTM. Thank you! Stay tuned for the final review please.

vuvova

see my comment about a test case. code change is ok.

vuvova · 2026-03-05T17:35:52Z

+
+let TERM=dumb;
+--error 0,127
+exec env MYSQL_HISTFILE=$histfile socat -t10 EXEC:"$MYSQL",pty STDIO < $MYSQL_TMP_DIR/mdev38494_in > /dev/null 2>&1;


I'd say it's a wrong test. The bug was that all concurrent mariadb clients used the same file. You don't test that, you test that don't use mdev38494_hist.TMP but they may all use the same different file name.

I'd say we don't need a test for a wrong symptom. Either it should be correct or no test at all (depending on how difficult it'd be to do a correct test).

MooSayed1 · 2026-03-06T03:32:35Z

@vuvova Thanks for the review.
I looked into writing a proper test with two concurrent interactive sessions, but it turns out to be quite difficult in MTR — there's no straightforward way to synchronize two background processes to hit the rename window at the same time.

So I've dropped the test for now and pusheded. so i'll wait for ur response if you have any suggestions for how to approach the test, or if the code-only change is fine as-is.
and thanks for all of u for helping.

grooverdan · 2026-03-18T03:05:56Z

+        /* Include PID in name to avoid rename collision when concurrent sessions exit. */
+        /* pid_t is signed but getpid() always returns a non-negative value (POSIX). */
+        snprintf(histfile_tmp, hlen, "%s.%lu.TMP", histfile,
+                 (unsigned long) getpid());


Need windows. From another area of code:

IF_WIN(GetCurrentProcessId(), getpid()))

create_temp_file (from mysys/mf_tempfile.c) might be a more portable way.

When multiple mariadb sessions exit at the same time, they all write to the same $HISTFILE.TMP then rename it to $HISTFILE. The second rename fails with Errcode 2 because the first already moved the file. Fix: include the process PID in the temp file name so each session uses its own unique path and no rename collision can occur.

MooSayed1 force-pushed the MDEV-38494-history-race branch from a1f4504 to f4d2ba2 Compare February 25, 2026 19:55

gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Feb 26, 2026

gkodinov requested changes Feb 26, 2026

View reviewed changes

MooSayed1 force-pushed the MDEV-38494-history-race branch from f4d2ba2 to 2d206ed Compare February 26, 2026 20:22

MooSayed1 requested a review from gkodinov February 26, 2026 20:52

gkodinov approved these changes Feb 27, 2026

View reviewed changes

gkodinov assigned vuvova Feb 27, 2026

vuvova requested changes Mar 5, 2026

View reviewed changes

MooSayed1 force-pushed the MDEV-38494-history-race branch from 2d206ed to c308525 Compare March 6, 2026 03:25

MooSayed1 requested a review from vuvova March 14, 2026 22:08

grooverdan reviewed Mar 18, 2026

View reviewed changes

vuvova force-pushed the MDEV-38494-history-race branch from c308525 to 8fe7820 Compare March 18, 2026 09:52

vuvova merged commit 52e9f3e into MariaDB:10.11 Mar 18, 2026
16 of 17 checks passed

Uh oh!

Conversation

MooSayed1 commented Feb 25, 2026

MDEV-38494: .mariadb_history rename race condition

Description

Release Notes

How can this PR be tested?

Basing the PR against the correct MariaDB version

PR quality check

Uh oh!

MooSayed1 commented Feb 25, 2026

Uh oh!

gkodinov commented Feb 26, 2026

Uh oh!

svoj commented Feb 26, 2026

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

gkodinov Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

gkodinov Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

gkodinov Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MooSayed1 commented Feb 26, 2026

Uh oh!

gkodinov commented Feb 27, 2026

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

vuvova left a comment

Choose a reason for hiding this comment

Uh oh!

vuvova Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

MooSayed1 commented Mar 6, 2026

Uh oh!

grooverdan Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

MDEV-38494: `.mariadb_history` rename race condition