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

[Jinoh Kang]

On 3/10/22 04:23, Zebediah Figura wrote:
> 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 just came up with another approach: why not just auto-bind the socket explicitly at send_socket?

We'll eventually call try_send() only after the send_socket server call, so binding the socket beforehand in the send_socket handler makes sense.

> 
> 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.


-- 
Sincerely,
Jinoh Kang