return-in-finally in multiprocessing/connection.py

[iritkatriel]

https://github.com/python/cpython/blob/7fd61607cd28ec466717c78adfb1eb5b63add1f0/Lib/multiprocessing/connection.py#L330C1-L340C68

This is clearly a bug, because the except: clause contains a naked raise, i.e., asks for the exception to propagate on. But the return in finally swallows that exception.

                    try:
                              ...
                    except:
                        ov.cancel()
                        raise
                    finally:
                        nread, err = ov.GetOverlappedResult(True)
                        if err == 0:
                            f = io.BytesIO()
                            f.write(ov.getbuffer())
                            return f
                        elif err == _winapi.ERROR_MORE_DATA:
                            return self._get_more_data(ov, maxsize)

Linked PRs

• gh-131233: remove return-in-finally in multiprocessing/connection.py #131416

[ncoghlan]

I don't believe this is actually a bug.

The logic as written:

• If an exception occurs while waiting for overlapped IO, cancel the overlapped IO request
• Regardless of whether an exception occurred while waiting or not, get the overlapped IO result
• If the IO operation was successful (in one of two possible ways), return a BytesIO instance containing the received data, even if an exception occurred while waiting for the operation
• If the IO operation wasn't successful, re-raise any raised exception

While it's a genuinely cryptic way of writing the code, I'm not seeing an immediately obvious way to rewrite it to eliminate the return-in-finally without duplicating the Win32 overlapped IO result and error code checking logic.

[iritkatriel]

Yes, I see - the return is conditional so it's possible that what you wrote was the intention.

Would this be a way to rewrite it?

                sentinel = object()
                return_value = sentinel
                try:
                    try:
                        if err == _winapi.ERROR_IO_PENDING:
                            waitres = _winapi.WaitForMultipleObjects(
                                [ov.event], False, INFINITE)
                            assert waitres == WAIT_OBJECT_0
                    except:
                        ov.cancel()
                        raise
                    finally:
                        nread, err = ov.GetOverlappedResult(True)
                        if err == 0:
                            f = io.BytesIO()
                            f.write(ov.getbuffer())
                            return_value = f
                        elif err == _winapi.ERROR_MORE_DATA:
                            return_value = self._get_more_data(ov, maxsize)
                except:
                    if return_value == sentinel:
                        raise

                if return_value != sentinel:
                    return return_value