BUG: race condition in Index.is_unique

[adbull]

Code Sample, a copy-pastable example if possible

Input:

from concurrent.futures import ThreadPoolExecutor
import pandas as pd

x = pd.date_range('2001', '2020')
with ThreadPoolExecutor(2) as p:
    assert all(p.map(lambda x: x.is_unique, [x]*2))

Output:

Traceback (most recent call last):
  File "bug.py", line 7, in <module>
    assert all(p.map(lambda x: x.is_unique, [x]*2))
AssertionError

Problem description

When calling Index.is_unique from multiple threads simultaneously, the wrong answer is returned.

Expected Output

Shouldn't raise.

Output of pd.show_versions()

INSTALLED VERSIONS

commit: None
python: 3.6.5.final.0
python-bits: 64
OS: Linux
OS-release: 4.14.14-200.fc26.x86_64
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: C
LANG: C
LOCALE: None.None

pandas: 0.23.0
pytest: 3.5.1
pip: 10.0.1
setuptools: 39.1.0
Cython: 0.28.2
numpy: 1.13.3
scipy: 0.19.1
pyarrow: None
xarray: None
IPython: 4.2.1
sphinx: 1.7.4
patsy: 0.5.0
dateutil: 2.7.2
pytz: 2018.4
blosc: None
bottleneck: 1.2.1
tables: 3.4.3
numexpr: 2.6.2
feather: None
matplotlib: 2.2.2
openpyxl: None
xlrd: 1.1.0
xlwt: None
xlsxwriter: None
lxml: None
bs4: 4.6.0
html5lib: 1.0.1
sqlalchemy: 1.2.7
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: 0.1.5
pandas_gbq: None
pandas_datareader: None

[jreback]

virtually nothing is threadsafe in pandas you have to be very careful. you are welcome to submit a patch.

[jreback]

see #2728

[batterseapower]

It's not surprising that modifying a frame while trying to use it from multiple threads is unsafe, but it's kind of weird that "read only" operations like is_unique can break. In my experience at least, this race in Index is the only part of Pandas that seems to go wrong when using a DataFrame in a read-only fashion from several threads. I guess there might be a few other initialised caches hanging around other parts of the codebase though.

[marberi]

Would this be an acceptable fix? I tested acquiring locks inside the __get__ method itself and
it did not work.

marberi@ad33858

[TomAugspurger]

We would need to measure the cost of acquiring the lock relative to the operation.

Do you know, if I do .is_unique multiple times, would the lock once, on the first call? And subsequent calls with return the cached value, without acquiring the lock?

Also, this feels like too low of a level for acquiring the lock, though I may be wrong about that.

[marberi]

Note that this will affect not only "is_unique", but all other methods which are using the same decorator. Also, it locks on all calls, which is excessive. That should be easily fixable, tho. You have a better suggestion of where to add the locking?

[TomAugspurger]

Maybe _ensure_mapping_populated? I'm not sure.

…

On Wed, Jun 5, 2019 at 3:14 AM marberi ***@***.***> wrote: Note that this will affect not only "is_unique", but all other methods which are using the same decorator. Also, it locks on all calls, which is excessive. That should be easily fixable, tho. You have a better suggestion of where to add the locking? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#21150?email_source=notifications&email_token=AAKAOIVFQ6YHT3AONMAZRF3PY5YVTA5CNFSM4FA3IYUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW66L5Q#issuecomment-498984438>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKAOIU7RIB2QCTN4HK2N3LPY5YVTANCNFSM4FA3IYUA> .

[rhshadrach]

I cannot reproduce on Python 3.12.

[batterseapower]

Seems unlikely that what you are observing is reliable, right? The only reliable (ie robust to thread scheduling changes) to fix this is to add a lock on the pandas side?

[batterseapower]

Just to be clear, the issue is being closed but the problem is not actually fixed, right?

Because _ensure_mapping_populated is still clearly not thread safe: https://github.com/pandas-dev/pandas/blob/main/pandas/_libs/index.pyx#L339

[rhshadrach]

Looking over this again, as mentioned above essentially none of pandas is thread-safe. Even operations that appear to the user to be read-only can modify caches for performance. I am negative making modifications to support thread-safety that have a performance impact unless there is a PDEP that can address (a) level of effort (b) single thread performance cost and (c) benefit across the entire pandas API.

Doing this in a one-off manner that has performance costs is not the appropriate way to address this.