Skip to content

Commit

Permalink
Resolve Heisenbug in StringHashTable._unique
Browse files Browse the repository at this point in the history
When processing an invalid Unicode string, the exception handler for UnicodeEncodeError called `get_c_string` with an ephemeral repr value that could be garbage-collected the next time an exception was raised.  Issue #45929 demonstrates the problem.

This commit fixes the problem by retaining a Python reference to the repr value that underlies the C string until after all `values` are processed.

Wisdom from StackOverflow suggests that there's very small performance difference between pre-allocating the array vs. append if indeed we do need to fill it all the way, but because we only need references on exceptions, we expect that in the usual case we will append very few elements, making it faster than pre-allocation.

Signed-off-by: Michael Tiemann <[email protected]>
  • Loading branch information
MichaelTiemannOSC committed Oct 15, 2023
1 parent 10cf330 commit c300b46
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 1 deletion.
1 change: 1 addition & 0 deletions doc/source/whatsnew/v2.2.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ Other
^^^^^
- Bug in :func:`cut` incorrectly allowing cutting of timezone-aware datetimes with timezone-naive bins (:issue:`54964`)
- Bug in :meth:`DataFrame.apply` where passing ``raw=True`` ignored ``args`` passed to the applied function (:issue:`55009`)
- Bug in Cython :meth:`StringHashTable._unique` used ephemeral repr values when UnicodeEncodeError was raised (:issue:`45929`)
- Bug in rendering ``inf`` values inside a a :class:`DataFrame` with the ``use_inf_as_na`` option enabled (:issue:`55483`)
- Bug in rendering a :class:`Series` with a :class:`MultiIndex` when one of the index level's names is 0 not having that name displayed (:issue:`55415`)
-
Expand Down
5 changes: 4 additions & 1 deletion pandas/_libs/hashtable_class_helper.pxi.in
Original file line number Diff line number Diff line change
Expand Up @@ -1128,6 +1128,7 @@ cdef class StringHashTable(HashTable):
use_na_value = na_value is not None

# assign pointers and pre-filter out missing (if ignore_na)
keep_rval_refs = []
vecs = <const char **>malloc(n * sizeof(char *))
for i in range(n):
val = values[i]
Expand All @@ -1144,7 +1145,9 @@ cdef class StringHashTable(HashTable):
try:
v = get_c_string(<str>val)
except UnicodeEncodeError:
v = get_c_string(<str>repr(val))
rval = <str>repr(val)
keep_rval_refs.append(rval)
v = get_c_string(rval)
vecs[i] = v

# compute
Expand Down

0 comments on commit c300b46

Please sign in to comment.