gh-139001: Fix thread-safety issue in pathlib.Path

[colesbury]

Don't cache the joined path in _raw_path because the caching isn't thread safe.

• Issue: Race condition in pathlib on 3.14t #139001

[barneygale]

It's a shame to lose the caching. Presumably something like this wouldn't work?

    @property
    def _raw_path(self):
        paths = self._raw_paths
        if len(paths) == 1:
            return paths[0]
        elif paths:
            # Join path segments from the initializer.
            path = self.parser.join(*paths)
            # Cache the joined path.
            paths[:] = [path]
            return path
        else:
            paths[:] = ['']
            return ''

[colesbury]

That doesn't seem to work, at least not in the free threaded build. I've added a test case.

I think we could probably make the caching thread safe with some extra work, but I'm not sure it's worth it. It looks like _raw_path is only used used to compute drive, root, and _tail, which are themselves cached, so I'm not sure there's any benefit for caching _raw_path.

[Zheaoli]

Hi @colesbury @barneygale, I think maybe we need to think about more about this PR

I made a benchmark

import pyperf
from pathlib import Path # or from pathlib_new which is a patched version 

PATH = "/" + "a" * 511
p = Path(PATH)

def bench():
    for i in range(100000):
        a = p.root
        del a

runner = pyperf.Runner()
runner.bench_func('bench', bench)

Run in 3.13.0 without free-thread

+-----------+----------+-----------------------+
| Benchmark | demo_old | demo_new              |
+===========+==========+=======================+
| bench     | 1.97 ms  | 5.60 ms: 2.84x slower |
+-----------+----------+-----------------------+

I think maybe we can use flag as a feature gate?

[Zheaoli]

By bad, previous bench have some mistake

import pyperf
from pathlib import Path # or from pathlib_new which is a patched version 


PATH = [chr(ord('a') + (i % 26)) for i in range(20)]
p = Path(*PATH)

def bench():
    a = p.root
    del a

runner = pyperf.Runner()
runner.bench_func('bench', bench)

Here's results

+-----------+------------+-----------------------+
| Benchmark | demo_old14 | demo_new14            |
+===========+============+=======================+
| bench     | 30.1 ns    | 29.4 ns: 1.02x faster |
+-----------+------------+-----------------------+

[Zheaoli]

Updated:

import pyperf
from pathlib_new import Path



PATH = ["abc123" for _ in range(200)]
def bench():
    p = Path(*PATH)
    a = p.root
    del a

runner = pyperf.Runner()
runner.bench_func('bench', bench)

Base on the code, I find that if the path level < 200 , this patch is fater than old way.

[colesbury]

@barneygale, are you okay with this change?

[barneygale]

As an illustration of the current caching behaviour, consider:

p = Path('/usr', 'local', 'foo', 'bar')
q0 = p / 'spam.txt'
q1 = p / 'eggs.txt'
str(p)
str(q0)
str(q1)

The current implementation would make these calls:

os.path.join('/usr', 'local', 'foo', 'bar')
os.path.join('/usr/local/foo/bar', 'spam.txt')
os.path.join('/usr/local/foo/bar', 'eggs.txt')

With this PR:

os.path.join('/usr', 'local', 'foo', 'bar')
os.path.join('/usr', 'local', 'foo', 'bar', 'spam.txt')
os.path.join('/usr', 'local', 'foo', 'bar', 'eggs.txt')

The extra arguments have a cost IIRC. And maybe that's fine, I just wanted to explain the current behaviour better.

[colesbury]

@barneygale, I've added a print statement to join in posixpath.py and I see the same thing on both main and the PR:

os.path.join('/usr', 'local', 'foo', 'bar')
os.path.join('/usr', 'local', 'foo', 'bar', 'spam.txt')
os.path.join('/usr', 'local', 'foo', 'bar', 'eggs.txt')

[barneygale]

Oh! If you move str(p) before the creation of q0 and q1 does that change?

[colesbury]

Yes, then you get the behavior you described

[barneygale]

If we can bring the optimization back in future that would be great, but for now it's better to have pathlib working in free-threaded python. Thanks for fixing this.

[miss-islington-app]

Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

[bedevere-app]

GH-139926 is a backport of this pull request to the 3.14 branch.

[colesbury]

@barneygale thanks for the review