Correctly indicate the input type limitations of `asyncio.write` and `asyncio.sendto`

[tecki]

Bug report

Bug description:

When using any of the write or sendto methods in the asyncio library with anything that is not an instance of bytes, bytearray or memoryview, one gets the error message "data argument must be a bytes-like object". This is even true if one does pass a bytes-like object.

The reason is that the code checks for exactly those three types. But there are many other types which are bytes-like. Until now it is only a documentation bug: one could just fix the error message.

However, it is also a real bug, because if you hand in a non-contiguous memory view to those methods, they will not complain, however they will fail, and that in a not-so-nice way, as the methods will happily put the data into a buffer to be sent out later, and at this later time it will fail.

Unfortunately I am not aware of a good test whether something is bytes-like. The test should be: does it pass well with the PyArg_Parse y* format type, as this is, according to the documentation, the recommended way to accept binary data, and also what the underlying functions in asyncio do.

CPython versions tested on:

3.11

Operating systems tested on:

No response

Linked PRs

• gh-138703: remove unnecessary and buggy type checks in asyncio #138737
• gh-138703: clarify data buffer requirement of asyncio.StreamWriter.write #139564
• [3.14] gh-138703: clarify data buffer requirement of asyncio.StreamWriter.write (GH-139564) #139570
• [3.13] gh-138703: clarify data buffer requirement of asyncio.StreamWriter.write (GH-139564) #139571

[sobolevn]

Can you please attach a minimal reproducer?

[picnixz]

I think we had a related issue about this but I don't remember well where. @kumaraditya303 do you remember? I'm pretty sure we discussed about the performance penalty that checking the protocol would have ensued

[kumaraditya303]

I have added a note in the docs for the requirement of C contiguous one-dimensional memoryview.

https://docs.python.org/3.13/library/asyncio-stream.html#asyncio.StreamWriter.write

[tecki]

@kumaraditya303 well, the note you added to the docs is wrong, at least given the current code. The correct note (given the current code) would be

The data buffer should be one of bytes, bytearray or memoryview. If it is a memoryview, it needs to be C contiguous.

That said, I would favor if your note was right and the code modified accordingly.

[picnixz]

Unfortunately I am not aware of a good test whether something is bytes-like

For checking if something is bytes-like, one could convert it into a memoryview and check for contiguous to see if the memory is contiguous. But this could be an expensive check. Alternatively, for non-standard types, we could do the following:

if not isinstance(obj, (bytes, bytearray, memoryview)):
	try:
		s = memoryview(obj)
	except (TypeError, ValueError):
		# raise an error
    else:
		if not s.contiguous:
			# raise again another error
		else:
			# continue with s.

This would only affect paths that are non-standards. However, if we really only want built-in buffer-like objects, we should document them differently.

[kumaraditya303]

Feel free to propose PR with better wording, the implementation should stay the same.

[tecki]

@picnixz I like your approach. Thinking about this I would go for:

data = memoryview(obj)
if not data.contiguous or not data.readonly:
    data = bytes(obj)

this is simpler, but does even more: the isinstance test is not actually needed, memoryview does that for us, and given that isinstance is actually not a very fast function, creating a memoryview might even be faster (I did not try). The try...except is also not needed, it is perfectly fine to just let memoryview raise an error.

I added the readonly check: if the user hands over non-readonly data (like bytearray), it may be modified later by the user, which is a bad idea. So in this case we should copy. If not (like the data is bytes) we do not need to copy, which may gain a little bit of time.

[picnixz]

Feel free to propose PR with better wording, the implementation should stay the same.

@kumaraditya303 Do you want to reject custom implementations of objects implementing the Buffer protocol (and that are contiguous)? That is, do you only want to restrict to instances of memoryview/bytes/bytearray but not those that are memoryview-compatible?

this is simpler, but does even more: the isinstance test is not actually needed, memoryview does that for us, and given that isinstance is actually not a very fast function, creating a memoryview

Yes and no. Converting a bytes to a memoryview is not necessary. We could convert it to a memoryview but it might be faster to use the bytes (or bytearray) object as is. That needs to be confirmed with benchmarks.

The try...except is also not needed, it is perfectly fine to just let memoryview raise an error.

We would want a better error message probably, that's why.

[kumaraditya303]

That is, do you only want to restrict to instances of memoryview/bytes/bytearray but not those that are memoryview-compatible?

Yes, for now I would prefer to restrict it to bytes, bytearray and one dimensional C contiguous memoryviews.