[PATCH v4 4/8] server: Defer postprocessing until after setting initial status in send_socket handler.

[Zebediah Figura]

On 3/4/22 13:16, Jinoh Kang wrote:
> On 3/5/22 02:51, Zebediah Figura wrote:
>> On 3/3/22 07:30, Jinoh Kang wrote:
>>> This allows the client to postpone the initial I/O until the server has
>>> queued the I/O request.  The server should perform the postprocessing
>>> only after the initial I/O has been done.
>>>
>>> In the case of send_socket, the manipulation of event flags shall
>>> ideally be done *after* (not *before*) the client has attempted the
>>> initial I/O, since the outbound queue status of the socket may change in
>>> the meanwhile.  Also, the implicitly bound address is available only
>>> after the send* system call has been performed.
>>>
>>> Signed-off-by: Jinoh Kang <jinoh.kang.kr at gmail.com>
>>> ---
>>>
>>> Notes:
>>>       v1 -> v2:
>>>       - pass around total size of data to be transmitted
>>>       - detect short write in send_socket_initial_callback
>>>       v2 -> v3: no changes
>>>       v3 -> v4: no changes
>>>
>>>    dlls/ntdll/unix/socket.c | 15 ++++++++++++++
>>>    server/protocol.def      |  1 +
>>>    server/sock.c            | 42 +++++++++++++++++++++++++++-------------
>>>    3 files changed, 45 insertions(+), 13 deletions(-)
>>>
>>
>> I still am failing to see the need to do this, especially by adding a new callback. What's preventing send_socket from working the same way as recv_socket?
> 
> As stated in the commit message, none of the actions in send_socket_initial_callback can be performed *before* the initial I/O.
> 
> In send_socket, "send() calls only clear and reselect events if unsuccessful."
> We can tell if the I/O has succeeded only after set_async_direct_result.
> Therefore, we can only clear and reselect events after the client tells us the initial I/O has been unsuccessful.
> 
Right, sorry, I'm not thinking this through, or reading apparently...

I still am not thrilled about adding a new callback, though, at least if 
it can be avoided. In this case I wonder if we can make use of 
sock_reselect_async().

The exact conditions (and timing) under which we need to clear the 
AFD_POLL_WRITE bit are not really clear. In particular the correct 
condition may not be "status != STATUS_SUCCESS" but "status == 
STATUS_PENDING || status == STATUS_DEVICE_NOT_READY". I can't off the 
top of my head think of any interesting cases where send() returns an 
error (other than EWOULDBLOCK) but subsequent calls can succeed...

But assuming that we only really need to clear flags when the kernel 
send buffer is full, I think the right thing to do would be to clear 
events if "async_waiting( &sock->write_q )".