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

[Zebediah Figura]

On 3/9/22 12:36, Jinoh Kang wrote:
> On 3/9/22 00:42, Jinoh Kang wrote:
>> On 3/8/22 03:11, Zebediah Figura wrote:
>>>
>>>
>>> On 3/5/22 02:24, Jinoh Kang wrote:
>>>> On 3/5/22 09:13, Zebediah Figura wrote:
>>>>> On 3/4/22 13:16, Jinoh Kang wrote:
>>>>> 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 )".
>>>>
>>>> We still need to account for nonblocking I/O (STATUS_DEVICE_NOT_READY), and also is_short_write, which is another indicator for a full send buffer.  Otherwise, it breaks ws2_32:sock:test_write_events.
>>>> Also, note the implicitly bound address.
>>>>
>>>> In any case I think the co-routine pattern is inevitable due to the client-server role split.  In this case, AFD.SYS can do both pre- and post-processing inside a single function, but wineserver can't block for the client to finish the I/O.
>>>>
>>>>
>>> Right. Maybe we could make use of async_set_completion_callback() instead? I think the only reason that can't work is if the sock name needs to be updated and visible *before* the I/O is necessarily complete.
>>
>> I'll test if Windows does update it before I/O completion.
> 
> Ok, here's the deal: it turns out Windows _does_ update the bound address before I/O completion.
> 

Right, okay.

My intuition still tells me we should avoid adding a new callback, and 
rely on the existing completion and reselect callbacks as much as 
possible. This is largely because we already have the need to execute 
coroutines, as it were, and reselect fills that need.

I think it's possible to accomplish that in this case relatively easily, 
by checking if there is a write async queued and trying to get the 
socket name if so. Although perhaps a more idiomatic alternative would 
be to replace all of the instances of "sock->bound" with a helper that 
tries to retrieve the socket name for unbound DGRAM sockets.

I'm not as confident about this as I'd like to be, and I don't like how 
complex the reselect callback can get. But adding more complexity to the 
async infrastructure just does not sound like a good idea.