#5088 Modernize Windows channel drivers

obsolete: 8.6b3
open
nobody
8
2012-07-25
2012-07-24
No

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.

Discussion

  • David Gravereaux

    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.

     
  • David Gravereaux

    • assigned_to: davygrvy --> nobody
     
  • David Gravereaux

    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.

     
  • Andreas Kupries

    Andreas Kupries - 2012-07-24

    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.

     
  • Andreas Kupries

    Andreas Kupries - 2012-07-24

    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>

     
  • Twylite

    Twylite - 2012-07-24

    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.

     
  • David Gravereaux

    If I remember correctly, the main notifier uses an alertable mechanism that supports calling a CompletionRoutine from it should that direction be chosen.

     
  • David Gravereaux

    TerminateThread() is not clean! You lose about 2k of stack memory that you can only get back by rebooting

     
  • David Gravereaux

    See MsgWaitForMultipleObjectsEx() with MWMO_ALERTABLE flag.

     
  • Alexandre Ferrieux

    • assigned_to: nobody --> davygrvy
     
  • Alexandre Ferrieux

    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.

     
  • Twylite

    Twylite - 2012-07-24

    @ 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()

     
  • David Gravereaux

    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

     
  • David Gravereaux

    • assigned_to: davygrvy --> nobody
     
  • David Gravereaux

    > 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

     
  • Twylite

    Twylite - 2012-07-24

    > 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 :)

     
  • Alexandre Ferrieux

    Just pretend we close when InExit+PIPE_ASYNC (fix by Twylite)

     
  • David Gravereaux

    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.

     
  • Twylite

    Twylite - 2012-07-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.

     
  • Alexandre Ferrieux

    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.

     
  • Alexandre Ferrieux

    • summary: io-29.33b fails on Windows by design --> Modernize Windows channel drivers