http://bugs.winehq.org/show_bug.cgi?id=17195
with only one significant bug - some memory corruption - the semantics are now correct in the namedpipe messagemode patch. i drew a diagram outlining the data structures / design http://bugs.winehq.org/attachment.cgi?id=19449 because it's so horrendous that even i want to throw up. assistance in finding the memory corruption would be really helpful. an assert is thrown after more than 4 short reads are performed on the same namedpipe, with this straaaange error: ead: file.c:631: server_get_named_pipe_fd: Assertion `wine_server_ptr_handle(fd_handle) == handle' failed. ead: server.c:628: server_get_unix_fd: Assertion `wine_server_ptr_handle(fd_handle) == handle' failed. wine: Assertion failed at address 0xf7ca8d7f (thread 000f), starting debugger... err:ntdll:RtlpWaitForCriticalSection section 0x7bc99ee0 "server.c: fd_cache_section" wait timed out in thread 000f, blocked by 0014, retrying (60 sec)
i'll try to write a simplified test because that's the "threadread" test on which this occurs.
i can't quite believe that all 5 tests can be passed [by reducing the number of short reads to less than 4]. it's been a bit of a "drunken walk" development style, i'm afraid :)
notes:
* in order to preserve message-mode semantics correctly, each message gets ITS OWN socketpair. the socketpair is used EXCLUSIVELY for that one message and is NOT re-used by the other end. so if it's client writing to server, one message, that's one socketpair. if it's server writing to client, one message, that's another socketpair. if client writes 20 messages to server, that's 20 new socketpairs, if at same time server writes 25 messages to client, and nobody's done any reading yet, that's a total of 45 outstanding socketpairs - 90 filedescriptors.
this is not a hoax. it's the only way i could think of that preserves messagemode semantics correctly.
* for each end - client and server - to be able to "block", waiting at any time, there MUST be at LEAST one socketpair in each of the queues. so, initially, that's what's done: one pair of socketpairs are created, one for the client, one for the server. on the FIRST write (to server, or to client), this initial "empty" socketpair must be written to; subsequent writes (to server or to client, each) must have a NEW socketpair created.
* on a read, a socketpair must "hang about" - MUST not be shut down - until there is a new one created (which happens only on a new write). in this way, multiple threads will always have a filedescriptor to block on.
* when a read has read all available data, the socketpair is marked as "finished" but is NOT shut down.
* on each write, if a new socketpair is created, then and ONLY then can "old" marked socketpairs be shut down.
* shutdown of read-sockets, on which [potentially many] reader[s] are waiting will result in an EPIPE. this is an ACCEPTABLE error IFF no data has been read yet, i.e. at the start of a new message. what happens, basically, is that the "old" socketpair - even though it had been used already by the previous reader - is what the thread "blocks" on (because you have to block on _something_). and on the _very_ first message, of course, you block on the specially-prepared initial socketpair, the one that had to be created and marked as "as-yet unwritten to".
* you can see at line 872 of ntdlls/file.c (after patch is applied) that oh whoopsie, EPIPE occurred, but we've not read any data at all, just grab another filedescriptor from wineserver and try again. in this way, wineserver hands out the next available message from the filedescriptor queue. if yet _another_ EPIPE occurs, then all that means is that another thread "got there first", wow big deal, around you go again.
* indicating that data has been read is performed using a special wineserver message "NAMED_PIPE_SUB_AVAILDATA" using server_set_named_pipe_info(). this horrible hack passes information to wineserver telling it to subtract the amount of data read in userspace from the filedescriptor data structure (fdqueue) representing that socketpair.
* indicating that data _will_ be available is performed again using server_set_named_pipe_info(), this time using a message "NAMED_PIPE_SET_AVAILDATA". the set is performed BEFORE the actual write, otherwise a reader will "unblock" and potentially try to set the available data to negative!!! this would be baaad.
* a critical section is performed around all uses of server_set_named_pipe_info() and server_get_named_pipe_fd(). this is NOT the same critical section as server_get_handle_fd(). i don't know if this makes any difference.
things that cannot be done:
* you cannot do the read or the write inside wineserver (server_named_pipe_read() will be deleted, later). the reason is complex to describe, but boils down to "blocking in userspace on the socket in order to wait for a message" and "non-blocking read on the same socket in order to get the data" are unfortunately mutually incompatible. one screws up the other if you set or clear O_NONBLOCK.
* you cannot send the length as a 4-byte "header" down the socket, to be read as a length and then that much data read out. if you have multiple readers blocking on the same filedescriptor, how do you make sure that only one of them reads the length?? they're all blocking on the same socket! send the length multiple times?? how do you know how many readers there are?? short answer: you can't. so - scratch that idea. [ok - maybe you could do a critical section, but you'd have to do one per file, and i'm not ready or willing to add an array of per-file-handle critical sections to ntdll without further discussion]
issues:
* multiple readers of the same socket, when reading data amounts that are larger than the SO_RCVBUF and/or SO_SNDBUF, could potentially end up corrupting the data because of overlaps in the reads. given that this is _already_ a problem, in the existing Wine code, i am not going to be dealing with this issue, in this iteration. particularly as solutions involve having to do per-file critical sections and YES that's a generic problem even on standard files not just pipes not just messagemode pipes ALL files.
* running out of socketpairs; protecting the message queue length. i believe there's a default message queue length in NT, but even so, the number of file descriptors this [necessary] design uses up is _horrendous_. one pair per message! but - tough luck. the options are pretty limited and constrained by the requirements.
so: if anyone says to me, "no this cannot work, you cannot _possibly_ have got correct messagemode semantics purely in userspace and without sending length fields, you _must_ do the reads and writes inside wineserver", then... well... that will be a good way to get me to unsubscribe from wine-devel. if anyone can think of any other ideas, or implementations, or optimisations, then... well... unless they're _really_ good ideas, they'll have to wait :) ideas such as using a separate socketpair for communicating "length" messages are welcome, if accompanied by well-laid-out explanations, because my brain is turning to jelly with this stuff and i could do with some light reading instead of working it out myself :)
l.