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.
Mike.
On Fri, Jul 3, 2009 at 5:49 AM, Mike Kaplinskiymike.kaplinskiy@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
On Sun, Jul 5, 2009 at 6:55 AM, Damjan Jovanovicdamjan.jov@gmail.com wrote:
On Fri, Jul 3, 2009 at 5:49 AM, Mike Kaplinskiymike.kaplinskiy@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.
-
Damjan Jovanovic -
Mike Kaplinskiy