gh-157242: Leave bytearray unchanged if resize() fails

[vstinner]

If bytearray.resize() or bytearray.take_bytes() fails, leave the bytearray unchanged.

If PyBytesWriter_Resize() fails, leave the writer unchanged.

Add a new internal _PyBytes_ResizeKeepOnError() function similar to _PyBytes_Resize() but leaves the bytes object unchanged on error.

• Issue: bytearray.resize() must not clear the object on error (memory allocation failure) #157242

[cmaloney]

It would be nice if the "guaranteed no global" case was also used for the PyBytes_FromStringAndSize(NULL, size); call. That the first parameter there must be NULL to avoid a global is unintuitive.

[serhiy-storchaka]

No need to add a new parameter, it saves nothing. _PyBytes_Resize can be implemented via _PyBytes_ResizeKeepOnError.

I wonder if we can simply change the behavior of _PyBytes_Resize.

[vstinner]

Please review the updated PR. I addressed reviews.

@maurycy found a fix for the memmove() code path which worried me. I applied his suggestion and added a test.

@serhiy-storchaka:

No need to add a new parameter, it saves nothing. _PyBytes_Resize can be implemented via _PyBytes_ResizeKeepOnError. I wonder if we can simply change the behavior of _PyBytes_Resize.

Thanks for the advice. I reworked _PyBytes_Resize(): for the in-place resize code path, no longer call _Py_ForgetReference() and _PyReftracerTrack() before PyObject_Realloc(). Only call them on success. With this change, I was able to easy implement _PyBytes_Resize() with _PyBytes_ResizeKeepOnError().

[vstinner]

It would be nice if the "guaranteed no global" case was also used for the PyBytes_FromStringAndSize(NULL, size); call. That the first parameter there must be NULL to avoid a global is unintuitive.

Maybe PyBytes_FromStringAndSize() documentation should be elaborated to explain that PyBytes_FromStringAndSize(NULL, size) can be mutated, whereas PyBytes_FromStringAndSize(str, size) must not be mutated?

Note: PR gh-156996 does fix PyBytes_FromStringAndSize() usage in bytearray. I don't try to replace this fix.

[vstinner]

bytearray.resize() and bytearray.take_bytes() have been fixed to no longer use a singleton: I merged main in my PR to get the PR gh-156996 fix.

[vstinner]

@maurycy: I modified resize() and take_bytes() to avoid memmove() usage if we would be unable to revert the bytesarray to its previous state on MemoryError. Does it look correct to you?

I also added more tests injecting MemoryError.

[maurycy]

@vstinner: Thank you. I will take a look more carefully later today.