BUG: memory leak on get_c_string
State: closed

 - [x] closes #16057
 - [ ] tests added / passed
 - [ ] passes ``git diff upstream/master --name-only -- '*.py' | flake8 --diff``
 - [x] whatsnew entry


Labels:
name: Performance
description: Memory or execution speed performance

Comments:
@jreback - this fixes the leak, but on a closer look, I'm not sure we were safe releasing the GIL in `StringHashTable` - the c strings are views back on the original python objects, so not threadsafe?
Here's a contrived case that creates a data race:

setup
```python
def f1(arr):
    return pd.unique(arr)

def f2(arr):
    arr.iloc[-1] = 'a new long string'

from concurrent import futures
import time
```

Then depending on timing, unique can change
```python
In [38]: x = pd.Series(np.arange(10**6) % 10**3).astype(str)
    ...: x.iloc[-1] = 'long string #1'
    ...: 
    ...: with futures.ThreadPoolExecutor(max_workers=2) as exc:
    ...:     fut1 = exc.submit(f1, x)
    ...:     fut2 = exc.submit(f2, x)
    ...: print(fut1.result()[-1])
a new long string

In [39]: x = pd.Series(np.arange(10**6) % 10**3).astype(str)
    ...: x.iloc[-1] = 'long string #1'
    ...: 
    ...: with futures.ThreadPoolExecutor(max_workers=2) as exc:
    ...:     fut1 = exc.submit(f1, x)
    ...:     time.sleep(2)
    ...:     fut2 = exc.submit(f2, x)
    ...: print(fut1.result()[-1])
long string #1
```









hmm it should be ok u r not manipulating python objects strings are fine
though since it's s pointer to an internal python object may be suspect


cc @wesm @cpcloud ?
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=h1) Report
> Merging [#16060](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/dd5cef560b2fc30aaad04e74134b3f52b64425ce?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/16060/graphs/tree.svg?width=650&height=150&src=pr&token=eZ4WkYLtcO)](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #16060   +/-   ##
=======================================
  Coverage   90.79%   90.79%           
=======================================
  Files         156      156           
  Lines       50537    50537           
=======================================
  Hits        45886    45886           
  Misses       4651     4651
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.56% <ø> (ø)` | :arrow_up: |
| #single | `40.44% <ø> (ø)` | :arrow_up: |


------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=footer). Last update [dd5cef5...8910efd](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=h1) Report
> Merging [#16060](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/dd5cef560b2fc30aaad04e74134b3f52b64425ce?src=pr&el=desc) will **increase** coverage by `0.05%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/16060/graphs/tree.svg?token=eZ4WkYLtcO&src=pr&height=150&width=650)](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #16060      +/-   ##
==========================================
+ Coverage   90.79%   90.84%   +0.05%     
==========================================
  Files         156      159       +3     
  Lines       50537    50794     +257     
==========================================
+ Hits        45886    46146     +260     
+ Misses       4651     4648       -3
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.63% <ø> (+0.06%)` | :arrow_up: |
| #single | `40.36% <ø> (-0.09%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY29tbW9uLnB5) | `90.68% <0%> (-0.35%)` | :arrow_down: |
| [pandas/io/formats/format.py](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvZm9ybWF0LnB5) | `95.02% <0%> (-0.1%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.64% <0%> (-0.02%)` | :arrow_down: |
| [pandas/io/formats/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvY29tbW9uLnB5) | `94.44% <0%> (ø)` | |
| [pandas/io/formats/excel.py](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvZXhjZWwucHk=) | `96.64% <0%> (ø)` | |
| [pandas/io/formats/css.py](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvY3NzLnB5) | `100% <0%> (ø)` | |
| [pandas/io/formats/style.py](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvc3R5bGUucHk=) | `96.36% <0%> (+0.07%)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `79.81% <0%> (+0.18%)` | :arrow_up: |
| [pandas/io/excel.py](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=tree#diff-cGFuZGFzL2lvL2V4Y2VsLnB5) | `80.62% <0%> (+0.88%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=footer). Last update [dd5cef5...e1b90eb](https://codecov.io/gh/pandas-dev/pandas/pull/16060?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Yeah, not sure this is actually an issue, not like mutating is ever thread safe.  I was thinking there could possibly be a use after free problem if one of the python strings got deleted during the `nogil` block, but I can't seem make it happen.
@chris-b1 your above example doesn't actually create a race. The actual conversion (IOW, the calling of ``get_c_string`` is WITH the GIL. After that we just have a pointer to the string, so even if there is mutation, it would have to be within a c-thread that itself mutates a ``PyObject`` (which is a no-no). So this looks ok.
from https://github.com/pandas-dev/pandas/blob/648ae4f03622d8eafe1ca3b833bd6a99f56bece4/pandas/_libs/hashtable_class_helper.pxi.in it looks like the GIL is always held when calling this function. It doesn't look like you are checking NULL though, so exceptions are going unchecked for a while. Perhaps you want `except NULL` in https://github.com/pandas-dev/pandas/blob/648ae4f03622d8eafe1ca3b833bd6a99f56bece4/pandas/_libs/src/util.pxd#L20?
Thanks @jreback, @wesm.  Added the `except NULL` - on the `StringHashTable` path we've pre-checked that the object array are all strings so it shouldn't ever hit, but as a general utility function I agree it should check.
thanks @chris-b1