Tcl Source Code

Given the following write-only refchan, the coroutine attempting to use such a channel yields once and writes to the channel once. After the coroutine yields a second time, the channel never becomes writable again and the coroutine is never entered again:

namespace eval refchan {
    namespace ensemble create
    namespace export *


    proc finalize {chan args} {
        namespace delete c_$chan
    }


    proc initialize {chan args} {
        namespace eval c_$chan {}
        namespace upvar c_$chan watching watching
        set watching {}
        list finalize initialize seek watch write
    }


    proc watch {chan args} {
        namespace upvar c_$chan watching watching
        foreach arg $args {
            switch $arg {
                write {
                    if {$arg ni $watching} {
                        lappend watching $arg
                    }
                    chan postevent $chan $arg
                }
            }
        }
    }


    proc write {chan args} {
        chan postevent $chan write
        return 1
    }
}


after 0  [list coroutine c1 apply [list {} {
    set chan [chan create write refchan]
    chan configure $chan -blocking 0
    chan event $chan writable [list [info coroutine]]
    for {set i 0} {$i < 10} {incr i} {
        yield
        puts $chan hello
    }
    set ::done 1
}]]
vwait done

The problem is that tclIO.c/Write adds the value from the first write into an internal buffer but takes no actions that result in another call to Tcl_NotifyChannel. In particular, the "write" command of the reference channel is never called because the result of IsBufferFull is "false".

This issue does not affect the "unchained" branch.

That excuse is so weak that my granny whipped its ass and called it Sally. Someone else on the core team can close this ticket if they really believe that [chan postevent] isn't intended to be asynchronous.

We had a meeting today. Today, it is my duty to close the tickets.
Sorry, the others are not available today. We have to share resources...

Thank you for your comprehension,
Harald

If someone other than Jan, Ashok, Harald, Steve, or Kevin closes this ticket and provides a rationale, I won't contest the closure any more. Jan, Ashok, and Harald are too invested in the argument, and Steve and Kevin don't have the expertise to assess it.

My answer is that chan postevent was always sufficiently documented as asynchronous, that no one has ever relied on it to be synchronous, that no one has ever shown a need for it to be synchronous, that it was always intended to be asynchronous, and that the campaign to suddenly label it as synchronous based on buggy behaviour, and without any evidenced need, is ridiculous. Ridiculous and harmful, particularly as it propagates broken refchans into the Tcl 9 release.

You all should submit a TIP to make chan postevent synchronous if you really believe that nonsense.

Ashok has gotten his facts wrong. I'll copy here one of my replies from to the mailing list.

chan postevent is the mechanism provided for a refchan to notifiy Tcl of events on the channel. At the C level, the mechanism provided to an event source for this purpose is Tcl_QueueEvent(), which places an event notification on the event queue. In other words, Tcl_QueueEvent() operates asynchronously by letting the event queue mediate notification. Since Tcl_QueueEvent() is asynchronous, chan postevent is clearly also intended to be asynchronous. Even the name "postevent" implies that. The command posts an event. And where does it post the event to? To the event queue of course.

Sorry Nathan !

This is expected behaviour.

As already pointed out by Ashok, the issue described here is, that a couroutine does not work synchroneously what is the issue underlying to this ticket.

Thank you for your comprehension, Harald

No one has pointed out any documentation explaining that this is expected behaviour. Until someone can reference such documentation, this report should remain open.

Sorry Nathan ! As informed on the core list:

• This is a RFE, not a bug. It will not work with the current design.
• The only way to change this is a TIP

In consequence, it is changed to a RFE and closed with the state "wont fix".

Thank you for your comprehension, Harald

I don't see any explanation in these comments about how this particular issue is invalid, so I'm reopening it.

This ticket is (still) invalid.

Closing this ticket as invalid and allowing [chan postevent] to remain a synchronous operation means that refchans will remain essentially broken in Tcl 9. Sad.

This ticket is invalid.

See: here for full motivation, which is approved by the TCT.

Ashok wrote, "A change to preallocate channel buffers, for example, would break this 'guess'."

No, it wouldn't, because the check is that the buffer is not empty (i.e. there has already been a write performed), and not full (i.e. and the write has not been flushed).

Ashok, is io-14.11.0 still failing on your system with [70a407c2453e5aa3] (which is the fix for [080f846fd58958ef])?

You mean this fix that has been in place since 2019? Not unless this failure has been occurring for the last 4 years.

I'm seeing the http-3.26.1 testcase fail sometimes on MacOS. Could this 'fix' be the cause of that?

See: https://github.com/tcltk/tcl/actions/runs/8534740843/job/23379729000

Adjusting the timer like that is one way to achieve rate control, but Ashok is arguing that rate control should be achieved by waiting for the device to signal that it's ready for more writes. The problem is that Tcl writes to a buffer without actually calling the [write] procedure that a refchan provides. The [write] procedure is the logical (and only) place for a refchan to signal that its ready for another write more, but because of this "write to a buffer instead of performing a write to the device" phenomenon, refchan must continuously signal that it's ready for a write, which means setting up a repeatedly-scheduled event.

I think one thing we can all agree on is that if a device signals that its ready for a write, and Tcl writes to a buffer instead of actually performing a write, then Tcl can assume that the device is still ready for a write, and can safely generate a "writable" event again. We should be able to develop an improved approach based on this assumption, and the new approach will eliminate the need for refchans to set up such repeatedly-scheduled events, and probably also eliminate the need for that CreateChannelScheduledEvent() function that sebres just pointed to.

Just few cents from my side about SYNTHETIC_TIMER - in my event-perf-branch from RFE [fdfbd5e10], I rewrote this synthetic timers with a scheduled event which can be retarded inside the handler if it is not yet ready.

TLDR: If you can find an acceptable way to eliminate that timer, then I'm all for it. Just don't revert this change wholesale. The situation calls for more effort than that.

Next...

I understand all of the principles that you've explained, and in theory agree. The thing is that Tcl provides a variety of knobs to achieve various behaviours, and also provides refchans, which must function even in the absence of external state changes that typical channels have. You're arguing from a matter or principle, but the trick is to accommodate all of these behaviours and contexts in practice. If you can eliminate that timer without breaking anything, than please do! What I'm arguing against is just reverting this change without providing any other fix, and without even providing an example of something that is broken and can't be fixed by any of the existing knobs. I have shown how to asynchronously connected by using [socket -async] and not configuring the channel as nonblocking until the "connected" event is handled. No one has made test io-14.11.0 reliably fail in the error case, so it's currently not a meaningful test. The idle queue starvation you mentioned was easily fixable separately. Until there is a concrete example showing something that needs fixing, the best way to continue is for any interested party to make improvements to existing code, and not to revert the change that fixed this issue.

I'm not fond of that timer either, and I've even provided an idea for taking steps towards eliminating it. Perhaps that got lost in the noise: The timer can be constrained to cases where a write (perhaps only to a buffer) has already occurred and an unfilled buffer exists. This would move things in the direction you're advocating by only employing the timer when it is really necessary. If you have time, you could do that work, or I'll do it when I have time, or maybe someone will come up with a way to eliminate the timer entirely. Let's work on it, and not just revert changes that have been in place for four years already, and that do solve issues like the one described in the current report.

All a refchan should have to do is use [chan postevent] to indicate that it is writable. Yes, refchans currently set up timers keep those writable events posting, but as you've just elegantly explained, that's a bad hack that violates the principles of asynchronous channels. So let's not enshrine that in the design, but instead get to work on the internals so that things remain simple at the script level, and we can continue to improve the situation in the implementation as intelligence and time resources become available.

Given that [socket -async] returns a blocking channel, it's more likely that the designers intended it to remain blocking until the connection is confirmed to be established.

The examples on the wiki were written over the course of the last 20 years, so I don't know which "bug" you're talking about, given that the changes Ashok and I are debating were introduced more recently.

> Almost all of the examples of [socket -async] on the wiki set the channel to nonblocking only after the connection is established. Seems like historically, people understood the right way to use [socket -async].

It's more likely that many people found out that setting [socket -async] from the start doesn't work right, because the Tcl implementation is buggy. Why setting up the channel cannot be asynchronious? Let's fix that!

If 14.11.0 was intentionally written to test this particular thing, then why is it only sporadically failing now, and only on some platforms? Also, why do the other tests that use the same pattern not fail then? Written as they are, only to a some-value-of-"works" standard, these tests are at least a little bit suspect. Almost all of the examples of [socket -async] on the wiki set the channel to nonblocking only after the connection is established. Seems like historically, people understood the right way to use [socket -async].

Regarding the fix for [ac7592e73c10d04b], care to explain how it "hides the real problem"? I think the fix gets a red herring out of the way so we don't waste time discussing it. The timer itself is debatable, but we might as well debate a correct implementation of it.

> Ashok, since you undid my change to socket test 14.11.0 ...

Ashok's action was IMHO fully justified. @nathan, please don't change a testcase which is written as-such on purpose. Ashok is not holding your changes hostage to win the argument: the goal here is to come up with a _real_ solution.

About the change from timer event to idle event: It's - indeed - actually the same, but it also hides the real problem. Not a good change to make directly on trunk, while discussion is ongoing. It's also not bad enough to start a 'commit war'.

I've created a new ticket, [ac7592e73c10d04b], for the idle queue starvation that Ashok provided an example of, and also fixed the issue by having tclIO.c/ChannelTimerProc() us the idle queue instead of the standard queue to reschedule itself.

Ashok, since you undid my change to socket test 14.11.0, it is now back to failing sporadically. If you want to prove your point, and also show that you're not just holding my changes hostage to win this argument, you'll make if fail consistently on all platforms so that it has meaning as a test.

I don't see anything wrong with delaying the writable event notification until a connection is established, but that should be done in such a way that this issue remains fixed. I have an idea for that: The issue here is that "writable" event notice happens once, then a "writable" event happens once but then never again because Tcl writes to a buffer instead of calling the "write" procedure for the refChan. I think it's pretty clear that refchan implementations shouldn't be burdened with the duty of maintaining timers, so somehow, Tcl must keep the "writable" event signaling until the "write" procedure of the refchan is actually called. The timer in UpdateInterest() is currently taking care of that. There is an opportunity to have Write() set up the timer instead, since that's the procedure that would call refchan if it a buffer was full. In that case, the timer-based "writable" event would only happen after some initial event has lead to a call to Write().

Nathan,

Regarding the GUI freeze, I put it here because the root cause is the exact same. If you trace through the code you will see ChannelTimerProc queueing the timer event which gets placed on the event queue, calling ChannelTimerProc queueing the event, which gets placed on the event queue ... Event queue never gets drained so the idle queue never runs.

I'd rather not have the same discussion under 3 tickets. If you find some other root cause and I'm mistaken, you can move it to another ticket.

/Ashok

@dgp, I had looked at that ticket you mentioned earlier. I think that is a much broader and trickier scope.

Nathan,

I have moved your change to 14.11.0 to a mistake branch.

This is the first time I have reverted someone else's changes, sorry, but it is simply not acceptable to change an perfectly functional test so as to not exercise a bug. The changes you made are not semantically equivalent.

You have repeatedly tried to justify your design by arguing that functional, valid user code to be changed. This is another example.

/Ashok

Ashok, that last comment belongs in its own ticket so it can be properly diagnosed and remedied separately if possible.

This may be off-topic.  Ignore or delete if it is.

I see much activity regarding events and channels, and I wonder if
any of it is relevant to ticket

https://core.tcl-lang.org/tcl/tktview/ef28eb1f15167156c898

If there's a chance to tackle that too while folks are already in the code.

c:\src\tcltk\wip\tcl\tests>tclsh86t ..\tests\refchan-async-test.tcl
1711899031
1711899032
1711899033
1711899034
1711899035
^C

c:\src\tcltk\wip\tcl\tests>d:\tcl\90\x64-debug\bin\tclsh90.exe ..\tests\refchan-async-test.tcl
1711899100
^C

c:\src\tcltk\wip\tcl\tests>d:\tcl\wip\x64-debug\bin\tclsh90.exe ..\tests\refchan-async-test.tcl
1711899197
1711899198
1711899199
1711899200
^C

d:\tcl\wip\x64-debug\bin\wish90.exe ..\tests\refchan-async-test.tcl

There has been an example on the wiki for 20 years already showing how to do port scanning in pure Tcl by connecting asynchronously to each port, and it wasn't broken by the fix for this issue because it never sets the channel to nonblocking: Port scanning in tcl.

I did address the port scanner issue by providing example code for asynchronously detecting when a port is open.

Also, in [1b9bd55b7af13673] I just fixed socket test 14.11.0 so that it will no longer sporadically fail on some platforms.

Ashok wrote, "It is about the use of writable events to asynchronously detect when a connection is open."

Again:

    set so [twapi::tls_socket -async www.example.com 443]
	    fileevent $so writable [list apply {chan {
		    #chan is now connected
		    fconfigure $so -blocking 0 
		    fileevent $so writable [list apply {chan {
			    #do stuff with connected channel
		    } $so]
	    }} $so]
	    vwait ...
    

That example asynchronously detects when a connection is open, so we're done here, right?

Yes, I incorrectly claimed that [chan configure ... -blocking] was redundant with [socket -async]. I forgot that [socket -async] returned a blocking channel. However, the fact that [socket -async] returns a blocking channel provides the solution to [080f846fd58958ef]: Don't immediately configure the channel to be nonblocking, and instead do this:

	set so [twapi::tls_socket -async www.example.com 443]
	fileevent $so writable [list apply {chan {
		#chan is now connected
		fconfigure $so -blocking 0 
		fileevent $so writable [list apply {chan {
			#do stuff with connected channel
		} $so]
	}} $so]
	vwait ...

Problem solved: The event will be triggered only when the channel is connected. Now, on to the issue of "writable" events on non-blocking channels. If a channel exists and is nonblocking, it's writable. Any other state of the channel, whether it is error state or connection state is not a factor. That's how nonblocking channels have always functioned in Tcl, and if you think about it, that's the way it must be: If something is blocking the writability of a channel, then it's no-longer nonblocking. That's why it has always been the case that for a nonblocking channel [puts] never fails, and also why "Tcl continues to buffer the data and writes it in the background as fast as the underlying file or device can accept it." ([puts] documentation), and why [close] is the operation that reports the error, if there was one: There's no other strategy that is practical.

Here's another example:

	set chan [socket -async <some address>]
	chan configure $chan -blocking 0
	puts $chan hello

Here, it's entirely possible that the socket will not be connected yet, but when [puts] executes it will never fail, nor should it. That's the point of configuring a channel to be nonblocking: It doesn't ever block. By definition, if a channel never blocks, it is always writable. Over in [080f846fd58958ef] Ashok wrote, "Under no circumstances should a file event writable callback be invoked unless the channel is actually open (writable) and the above does not make that check." As we see in the example above, that's wrong: [puts] doesn't fail, and if it doesn't fail, then the channel was writable. If the channel is writable, there's no problem triggering a corresponding event on the the channel. Not only that, but the fire-and-forget aspect of nonblocking channels is very convenient, particularly at the script level.

I incorrectly claimed that the test 14.11.0 failure we were talking about was the case where it produces an error. The actual situation is that the test fails if an error does not occur, but I believe it is by design that sometimes there is no error. The documentation for [close] states,

"If the channel is nonblocking and there is unflushed output, the channel remains open and the command returns immediately; output will be flushed in the background and the channel will be closed when all the flushing is complete."

That means that for systems that take their sweet time figuring that a connection can't be completed, [close] will in the meantime return without error, which in turn means that test 14.11.0 is only testing circumstance rather than anything meaningful. Perhaps that test should be deleted, or perhaps a pause should be introduced to let the system finish meditating on the state of the connection.

I believe that the existing fix for this issue is theoretically correct, and should remain in place. A few years ago I submitted issue [67a5eabbd3] and fixed it. Prior to this, refchans were thoroughly broken in an subtle way, and I believe this caused refchan programmers over the years to throw up their hands and just walk away from refchans, or to invent twisted cargo-cult approaches to keeping their refchans responsive. That issue is related to this one. Not taking the approach that has now been established for triggering writable events on asynchronous channels risks making them unusable again in ways that are too subtle for most programmers to ascertain, and in my experience means that we'll just keep playing whack-a-mole with issues related to handlers not ever getting triggered even though the should be.

I'm not in favour of changing the default buffering for refchans. That would be a knee-jerk reaction to a particular problem rather than a well-considered design, and in issue [080f846fd58958ef] there is so far no actual example of a broken script that would warrant such a change.

> Jan, I do not think you have fully understood the issue

Indeed, I don't understand 100% of all details, but globally, yes I do.

Switching buffering off for refchan's by default sounds OK to me: Buffering is meant to assure that the channel can continue operating if the next step is blocked. So, in a chain, if one of the elements has buffering on, all the other channels are able to operate through that.

Also, delaying the timer even until after the channel is fully open sounds OK to me. But it would be good to assure it doesn't break any outside channel implementation. Better be a little bit too careful here.....

Anyway: discussion about this is a good thing happening! You are bringing in a lot of arguments and examples! That's constructive, and - hopefully - helps finding a solution.

With respect to side effects with my branch setting refchan buffering to none by default, I have not seen any after experimenting for a while. All tests pass on Win and Linux and for the most obvious consequence - performance - there is no discernible difference:

With current trunk:

```
% timerate -calibrate {}
0.08983439299958879 µs/#-overhead 0.090688 µs/# 21524429 # 11026859 #/sec
% set fd [tcl::chan::memchan]
rc0
% timerate {puts -nonewline $fd x}
7.680620 µs/# 128693 # 130197 #/sec 988.442 net-ms
% seek $fd 0
% string length [read $fd]
128693
```

With the -buffering none based solution

```
% timerate -calibrate {}
0.09071036436181426 µs/#-overhead 0.091423 µs/# 21351367 # 10938200 #/sec
% set fd [tcl::chan::memchan]
rc0
% timerate {puts -nonewline $fd x}
7.687302 µs/# 128568 # 130084 #/sec 988.341 net-ms
% string length [read $fd]
0
% seek $fd 0
% string length [read $fd]
128568
```

I suppose one might expect this. Buffering is really meant to mitigate the cost of disk i/o and system call transitions. There is none with reflected channels.

An alternative, but similar effect, would be to set buffersize to 1 but the related code is a little more convoluted and I don't think offers any benefit over -buffering none.

I'm inclined to favor the -buffering none solution. It handles not the socket test and Nathan's coro test. Docs will have to note refchans do not get buffering by default. Also, we would have to decided whether the application can change the buffering mode (I would say yes as long as the potential issues are noted in the documentation. If that is not desired, we could simply force the buffering to be -none and ignore / raise error on any requests for change).

The refchan issue described here is far more serious than the failure of test socket-14.11.0, because in a situation like the one illustrated by that test, at least the script becomes aware that the channel is broken, and has the opportunity to respond somehow. In contrast, any script that falls victim to the refchan issue described in this ticket, it has no recourse, and simply fails silently and irretrievably.

There are many ways in which channel can become broken, and a socket failure to connect is for practical purposes not much different than any of those other failure modes. In my opinion, "async" should mean async from start to finish, and the script itself can decide how to handle the various possible failures in an asynchronous way.

In the meantime there are ways to avoid the problem Ashok illustrated with twapi in [080f846fd5]. For one thing, the example is a little strange:

      set so [twapi::tls_socket -async www.example.com 443]
      fconfigure $so -blocking 0
  

The socket is opened in asynchronous mode, but the for some reason it is still necessary to set the blocking mode of the resulting channel to 0. Why the redundancy? I would guess that if the "-async" argument were removed, then twapi would wait for the connection to complete, and then the rest of the code would continue to function asynchronously:

      set so [twapi::tls_socket www.example.com 443]
      fconfigure $so -blocking 0
  

The same is true for socket-14.11.0. The documentation for the "-async" option to [socket] has long described the possibility: "This means that the socket will be created immediately but may not yet be connected to the server, when the call to socket returns."

So even just following the documentation will avoid the issue Ashok has raised. I would not revert this fix. There may well be a more satisfying fix, and if it is found, it can of course replace this one.

@nathan, can you repond? Would it be best to revert this 'fix', and find a better one?

I still frequently (though not always) see test socket-14.11.0 fail.

The issues mentioned may have been fixed by subsequet work.  Are there any outstanding issues related to this ticket, or can it be closed now?

Failures of chan-io-44.6 may be arising here too.

The test socket-14.11.0 started failing with this change. Should we resolve
here or I open a new ticket?

Further explanation: The documentation for [chan event] states that a channel is considered to be writable if at least one byte of data can be written to the underlying file or device without blocking, or if an error condition is present on the underlying file or device. However, for better performance, Tcl produces writable events until the internal buffer is flushed. This fix extends this behaviour to reference channels.

Fixed in [d0dd6d19a41ae7af].