Tcl Source Code

View Ticket
Login
Ticket UUID: 3547994
Title: Modernize Windows channel drivers
Type: Bug Version: obsolete: 8.6b3
Submitter: ferrieux Created on: 2012-07-24 19:12:18
Subsystem: 25. Channel System Assigned To: nobody
Priority: 8 Severity:
Status: Open Last Modified: 2012-07-25 17:00:33
Resolution: None Closed By:
    Closed on:
Description:
As observed after  solving 3545365, on Windows the design of the Pipe driver prevents from cleanly cancelling a background flush. Indeed, the write-side worker thread uses synchronous WriteFile()s which cannot be cancelled from another thread across versions of Windows.
The aim of this ticket is to find *any* method to do that as cleanly as possible, with minimal ifdeffery, so that TIP #398 can claim to be working on Windows too, and io-29.33b stop from failing there.
User Comments: ferrieux added on 2012-07-25 17:00:33:
Confirmed by Jos (jdc), trunk+winpipe.patch , Win7, MSVC10, both 32 and 64-bits builds.

Committed to trunk.

Renaming the bug to reflect the more ambitious plan sketched in the comments.

twylite added on 2012-07-25 16:16:25:
I've tested Alexandre's revised patch as follows:
(1) Build with MSVC9; OPTS=threads,symbols; 32-bit EXE running on Windows XP 32-bit.  Passes full Tcl test suite, plus my 'iobg.test'.
(2) Build with MSVC10; OPTS=threads; 32-bit EXE running on Windows 7 64-bit.  Passes full Tcl test suite, plus my 'iobg.test'.

Both builds are trunk [3bbd5361] with branch bug-3545363 merged in, my revised td-patch-3545363-3.diff patch from that bug applied, and the revised winpipe.patch from this bug applied.

I think we're done here.

davygrvy added on 2012-07-25 04:24:00:
> I will look into this if/when I find the time, and will prepare to stand corrected :)

Just read through http://iocpsock.cvs.sourceforge.net/viewvc/iocpsock/iocpsock/iocpsock_lolevel.c?revision=1.116&view=markup to see how it is done right.  While reading, imagine IOCP_RECVMODE_ZERO_BYTE is always set as the code does too much tweaking and needs to have all the other mode logic removed.

davygrvy added on 2012-07-25 04:19:55:
I found posting a zero sized buffer to act as an alert followed by a blocking WSARecv() did the best speed and matched how the core works.  The only problem was how disconnected the socket's winsock buffer was compared to channel's as [fconfigure $sock -buffersize] doesn't forward to setsockopt(,SOL_SOCKET,SO_RCVBUF,...) to keep it the same.

ferrieux added on 2012-07-25 04:09:18:

File Added - 449727: winpipe.patch

ferrieux added on 2012-07-25 04:08:43:

File Deleted - 449719:

twylite added on 2012-07-25 03:52:35:
> They sure do! Call it with a zero size buffer. Pure notification

I suspected that approach may be possible, but it seemed rather invasive, and I hadn't investigated it.  It would allow the use of an alertable wait state that blocks window messages (preserving modal behaviour).

I will look into this if/when I find the time, and will prepare to stand corrected :)

davygrvy added on 2012-07-25 03:39:18:
> IoCompletionPorts don't provide a way to notify you that a stream has pending data.

They sure do!  Call it with a zero size buffer.  Pure notification

davygrvy added on 2012-07-25 03:33:40:
The only problem I had rewriting sockets to completionports was that some errors happen late, and notifications of closed did not repeat itself.

It is easier than you think.

A blocking write is easy to do.. just hold until the completion thread comes back.  And BTW, you only need one completion thread for the entire process.

See the iocpsock project here on SF

twylite added on 2012-07-25 03:26:51:
@ David: moving all IO to overlapped is a problem because of the impedence mismatch between the Windows and Tcl eventing models.  Tcl expects to live in a level triggered world.  Windows is primarily edge triggered.  That's why we have these worker threads that turn the edge trigger into a level that a Tcl event check proc can handle.

> If I remember correctly, the main notifier uses an alertable mechanism that
> supports calling a CompletionRoutine from it should that direction be
> chosen.
> See MsgWaitForMultipleObjectsEx() with MWMO_ALERTABLE flag.

Mmm.  Off-topic, but I'll bite: You can't actually use OVERLAPPED and completion routines, because of the impedence mismatch.  In *nix you call select() to see if something is readable or writable.  In Windows you perform an overlapped ReadFile() or WriteFile() and get notified of the result.  IoCompletion ports don't provide a way to notify you that a stream has pending data.

If you want level-based notification in Windows you either use select() (as in *nix, with very slow pipes as the alert mechanism), or you use WSAAsyncSelect (similar to select() but you wait on events with WaitForMultipleObjects), or you use WSAEventSelect which sends messages to a widows in a thread.  The former two have limited scalability - in particular WaitForMultipleObjects has a limit of 64 objects and MS recommends breaking larger wait lists into groups and worker threads.

So I did a quick port of tclWinSock.c to use the notifier window instead of the cothread window.  Works marvellously, until you want to simulate blocking IO at the Tcl level.  At that point you need to wait on an event, without blocking window messages from WSAEventSelect (which will set the event), with blocking any window messages that could cause the blocking IO to be non-modal.  E_CANT_DO.

It _should_ be possible to rework the Windows notifier, sockets and pipe code to use just one additional thread per process.  It's a major change.

It's all great right until you want to do blocking (from Tcl's perspective) socket IO in the main thread.  Then you can't use MsgWaitForMultipleObjectsEx()

ferrieux added on 2012-07-25 03:19:46:

File Added - 449719: winpipe.patch

ferrieux added on 2012-07-25 03:18:03:
While I can only sit back and watch you guys do magic with Win32, I'm humbly attaching a simple patch that allows TIP#398 to work as documented, simply by handling separately in PipeClose2Proc: (1) process-exit: just pretend we close, do nothing; (2) thread-exit: do as usual (ie wait blockingly for the writer thread to finish), which has been okay for decades and does not concern 398. Please test.

davygrvy added on 2012-07-25 03:13:26:
See MsgWaitForMultipleObjectsEx() with MWMO_ALERTABLE flag.

davygrvy added on 2012-07-25 03:07:42:
TerminateThread() is not clean!  You lose about 2k of stack memory that you can only get back by rebooting

davygrvy added on 2012-07-25 03:06:32:
If I remember correctly, the main notifier uses an alertable mechanism that supports calling a CompletionRoutine from it should that direction be chosen.

twylite added on 2012-07-25 02:58:55:
Hi.  Imho the TerminateThread is the way to go.  It's actually quite clean, and I'll have a patch available just as soon as patch stops preventing me from applying a diff that I really need in place.

The FILE_FLAG_OVERLAPPED will have a broad impact on the rest of the implementation, and probably introduce a whole set of new bugs (including subtle synchronisation bugs -- look at what goes on in tclWinSock.c to support blocking operations).

Tcl dropped support for Windows 2000 some time ago (it no longer compiles against the 2000 Platform SDK), which means the easliest Windows targetted is XP, which does support pipes in WaitForMultipleObjects.  That would allow the number of cothreads to be reduced, but at least 1 is still required.

andreas_kupries added on 2012-07-25 02:51:15:
Heh, x-ed.

Oh, and nearly last comment in 3545365 was, by ferrieux:

<quote>
The FILE_FLAG_OVERLAPPED route seems to be the way to go; however, if we do
that, we might wonder whether it is still necessary to have worker
threads... ISTR that some older versions of Windows were unable to include
a pipe in a WaitForMultipleObjects; I would appreciate an update on this:
if all currently supported Windows support it, the we could go back to a
purely event-driven async IO scheme similar to the one in unix.
</quote>

andreas_kupries added on 2012-07-25 02:50:21:
Hi David. Nice that you are still around.

Even if you do not do much core hackery I hope you will not mind if we might ask you questions. I still remember iocpsock, and maybe we can glean something from there for the pipe driver. Issue might be for which versions of Win* this is supported.

davygrvy added on 2012-07-25 02:49:10:
Here's a worthy project..  Let's move all I/O to overlapped.  pipe, file, sockets, console, etc.

That'll take "a large block of time" but would be worth it to get us up to current/modern practices.  My overlapped socket code could share quite a bit with the rest of the core.

davygrvy added on 2012-07-25 02:36:40:
Oh, I remember this problem well :)

Have a look at CancelSynchronousIo() and CancelIo().  I'm not much use these days at core hackery, so I can't be assigned to this task.

Attachments: