AcceptEx Proposal

[Mike Kaplinskiy]

On Sun, Jul 5, 2009 at 6:55 AM, Damjan Jovanovic<damjan.jov at gmail.com> wrote:
> On Fri, Jul 3, 2009 at 5:49 AM, Mike
> Kaplinskiy<mike.kaplinskiy at gmail.com> wrote:
>> I have been thinking about the correct way to implement AcceptEx, and
>> I was looking for comments about the approach. Note I know all of
>> these cases will require tests, I'm just listing them for problems
>> someone might see.
>>
>> Wineserver changes:
>>  - add an async* to the socket structure to track this socket's
>> request on the listening socket's queue. (set = null on accept)
>>    - Can anything do anything wrong while trying to remove the async
>> when the accepting socket is closed before the accept event is
>> received? (removing another socket's async request)
>>    - When/how should we handle setting this (create a new request
>> type (that queues & sets), create a new request type (that just sets)
>> or somehow splice into sock_queue_async? - my vote for first, as much
>> as I hate it)
>>  - change accept_socket to use two sockets instead of returning the
>> accepted socket
>>    - Problem: we have to destroy the fd of the old socket. Can
>> anything queue async reads/writes before the overlapped acceptex
>> returns? Do we care that the fd of a socket object gets switched (you
>> lose internal state, but I don't think you can have a lot of state on
>> an unbound socket)
>>    - Deferred sockets are a pain: 2 clients attempt to
>> connect->accept->defer->acceptex=which socket does it return? Also if
>> this order happens, and we have to destroy the fd as above, would the
>> application possibly care?
>>    - need to implement SO_UPDATE_ACCEPT_CONTEXT and have accept()
>> call it right after getting an accept.
>>  - if we close the listener socket, what happens to sockets waiting
>> for clients on the accepting socket? Can we reuse these sockets?
>>
>> ws2 changes:
>>  - exactly the same as the previous attempt, just using the above requests
>>  - more strict checks are needed on the second acceptex socket (need
>> to check family, bound state)
>>  - accept: a = create_socket(), accept_socket(listener, a),
>> ioctl)SO_UPDATE_ACCEPT_CONTEXT, a, &listener);
>>
>> I just started looking at wineserver, so don't be surprised if
>> something above doesn't make sense.
>
> I've just sent in a patch
> (http://www.winehq.org/pipermail/wine-patches/2009-July/075391.html)
> with some tests for AcceptEx. I hope to write many more tests over the
> next few days.
>
> After 94 people on the CC list, 132 votes, 376 comments, and almost 2
> years, it's now way past time for bug 9787 to die.
>
>
>> Mike.
>>
>>
>>
>
> Good luck Mike, and thank you in advance
> Damjan
>

Damjan,

That actually helps a lot. Some comments about the test: INFINITE
isn't really good to use in tests; there is some sort of TEST_TIMEOUT
constant wine uses for WaitForSingleObject. Also, it would be great if
you could write some sort of overlapped_server (I believe Scott [the
original AcceptEx patch] did this a little while ago) so that we can
test with various clients.

As I said above, there are a corner cases with CF_DEFER + AcceptEx and
AcceptEx + closesocket(listener) + connect(acceptor). Would you mind
writing some test cases for that too?

Regardless, since there were very little comments on this proposal, it
is now live code: http://bugs.winehq.org/show_bug.cgi?id=9787, at the
end.

Mike.