Recent commit to trunk caused multiple test failures due to errors raised by [close].

chan-io-53.5
event-11.5
io-53.5
socket_inet-2.12
socket_inet6-2.12
socket-14.2
socket-14.7.2
socket-14.8.2
socket-14.12
socket-14.15

> That said, I put the error message generation in the channel driver routines
> because that's what the documentation says should be done

Yeah, I don't think this documentation is entirely consistent with the current implementation of most channels.

> I think I'd be happier if we made that bump
> to TCL_CHANNEL_VERSION_6 as part of the post-vote implementation review.
> We would make version 6 available in 8.7, and allow only it to go forward to Tcl 9.

Sorry, but I don't think so. Please have a look at tclsqlite, which has now a TCL_CHANNEL_VERSION_5 channel. I would prefer a single channel version number which continues to work with Tcl 8.5 up to 9.0, with only one little restriction: If you want to make it compatible with both 8.5 and 9.0, it should have both a closeProc and a close2Proc. And if it has a seekProc, it should have a wideSeekProc too. That's how it's described in the TIP, and it's 100% intended like that.

With the trunk working again, it's less critically important, but I do have one
reservation in hindsight about TIP 562, which I understood in simple terms as
dropping support for channel type versions 1 through 4, but retaining support
for version 5 channels.

That's not really what the TIP did, though. Some existing version 5 channels
define a closeProc and may expect it to be called, but after TIP 562, it will
not be called any more.  We really created a version 6 of the channelType
but failed to bump the number.  I think I'd be happier if we made that bump
to TCL_CHANNEL_VERSION_6 as part of the post-vote implementation review.
We would make version 6 available in 8.7, and allow only it to go forward to Tcl 9.

That's what we're already really doing; this would just label it more accurately.

I'd rather defer to someone with a better sense of channels than me on the main
question of what errors should be filtered away, and where that choice should
be located.

It seems risky to expect all of these decisions to land in the central generic code
instead of being in the drivers. What other channel types might raise ENOTCONN?
Does it always mean something that can be ignored on close? I just don't know.

Lurking in this is the fact that we've fixed a bug (I think) that was skipping over a
half-close attempt on an entire type of channels. Whether that matters to anyone
is probably something we need an alpha release to discover.

It's better to have passing tests instead of failing ones.

That said, I put the error message generation in the channel driver routines
because that's what the documentation says should be done. Why else pass
in an interp argument to them?

http://www.tcl-lang.org/man/tcl8.6/TclLib/CrtChannel.htm#M10

Thanks, Don!

Indeed, the error-handling was not quite correct. The TIP #562 implementation was not to blame, since it just ignored all errors from close2Proc when doing a full close. Later I changed that to check just for EINVAL, but that - apparently - wat not sufficient.

The best solution is - in my view - filter out both EINVAL and ENOTCONN, all other errors I would like to let it go through as-is.

My fix committed now. Can you live with this solution?

Thanks!

What a mess.

Part of Tcl_Close in Tcl 8.6 sources looks like this:

    /*
     * If this channel supports it, close the read side, since we don't need
     * it anymore and this will help avoid deadlocks on some channel types.
     */

    if (chanPtr->typePtr->closeProc == TCL_CLOSE2PROC) {
        result = chanPtr->typePtr->close2Proc(chanPtr->instanceData, interp,
                TCL_CLOSE_READ);
        if (result == EINVAL) {
            result = 0;
        }
    }

Based on the comment, the value TCL_CLOSE2PROC is being used to signal the existence of a half-close supporting close2Proc in the Tcl_ChannelType. This
is true, but this is not the only way such a thing can be present. Surely it
is more reliable and correct to just test the close2Proc entry itself. Arguably
this has always been a bug.

But since the code has never been written that way, it has always skipped
over the half-close attempt for any channel that provides a real closeProc,
which the unix "tcp" channel does. At a minimum, our own channels are bug
dependent.

With recent changes that ignored the possibility of independent closeProc
and close2Proc entries in the channel type, we now have a half-close getting
called on "tcp" channels for the first time as part of a typical [close].

When the socket connection isn't good, the half-close can raise ENOTCONN.
While Tcl_Close filters out EINVAL, it lets ENOTCONN through, so for the
first time with these changes we have [close] raising errors. This is a
script level incompat. Unwelcome. Somewhere we need to restore the behavior
that detection of a bad connection during a [close] doesn't cause [close]
to throw an error (especially not one with NO error message or code!)

Recent commits changed the handling of a close on a Tcl_ChannelType
that defines both a closeProc and a close2Proc.  The "tcp" Tcl_ChannelType
in tclUnixSock.c is such a case.

Testing fix now.

Same failures on core-8-branch in a TCL_NO_DEPRECATED build.