Tcl Source Code

In the course of the investigation for [Bug 780533], I
stumbled upon the following weirdness of the implementation (quoting
myself since nobody answered that part ;-):

> A side-note about the implementation: if I read correctly, the
> asynchronous [fcopy] does its first chunk synchronously (in
> non-blocking mode). This is a bit unusual in event-driven style: setup
> procedures just set things up, the real work being done only from
> within [vwait]. While here, a small bit of the copy may be done
> immediately, even if the program doesn't call vwait before some
> time...
> Why is it done this way ? To factor code between sync and async cases ?
> Do we want it to be this way ? Could it explain nasty context-dependent bugs ?

Now, I'd like to pour in additional evidence that this is just plain wrong.

The idea is that, since [fcopy -command] is both sync and async, an
error raised in the callback may pop out either synchronously (subject
to a catch on [fcopy] itslef) or asynchronously (subject to bgerror):

1) Fast transfer (2 bytes):

 % proc plop args {puts stderr PLOP:$args;set ::done 1;error yo}
 % set f [open ha r]
 % set n [open nul: w]
 % fconfigure $f -translation binary
 % fconfigure $n -translation binary
 % fcopy $f $n -size 2 -command plop
PLOP:2

 % puts $::errorInfo

   while executing
"fcopy $f $n -size 2 -command plop"

(notice also that the error status is lost, while errorInfo is OK. Yet
another bug.)

2) Slow transfer (20000 bytes)
# do the same with -size 20000 and a file longer than that
 - fcopy returns control without PLOP
 - you need to enter a vwait to get the PLOP

 (yuck -- the error is lost again; but if it wasn't, it'd be a bgerror)

Questions:

 - Is the disabling of errors in the callback intentional, in order to
hide that catch/bgerror dichotomy ?

 - If not, would you accept a patch straightening both issues simultaneously by:

      - removing the sync part of [fcopy -async]
      - re-enabling errors in the callback (hell if I know how to do
this right now)

This way, both examples above would end up in bgerror.
(Needless to say, I volunteer for the patch)

Logged In: YES 
user_id=75003
Originator: NO

Well, the final patches are not quite final. For 8.5/Head copy the nerw testcase to chanio.test and tweak a bit for use of the 'chan' command. committed to 8.4, 8.5, and Head. All passing the extended testsuite.

File Added - 273099: final-fcopy-async-patches.tar.gz

Logged In: YES 
user_id=75003
Originator: NO

File Added: final-fcopy-async-patches.tar.gz

File Added - 273031: fcasync.tcl

Logged In: YES 
user_id=496139
Originator: YES

Attaching a test script.
It tests two separate things: (1) suppression of the synchronous part (2) bgerror.
What you will see, is that (1) was clearly not working before the patch (detected by the growth of the output file), but (2) worked anyway because of the other bug prevented the callback from firing in the synchronous part ! Of course, my previous patch made (2) reappear :-)

In summary:
- use stock Tcl to see 1=FAILURE 2=SUCCESS
- use [-command] patch to see 1=FAILURE 2=FAILURE
- use this patch to see 1=SUCCESS 2=SUCCESS


File Added: fcasync.tcl

Logged In: YES 
user_id=75003
Originator: NO

Confirm that patched Tcl core passes the regular testsuite. Tested for 8.4, 8.5, Head, non-threaded, non-debug build, linux.

Alexandre, do you have a small test script we could add to the testsuite, which demos the new bgerror handling (and would fail pre-patch)?

File Added - 272963: fcopy-async-tcl8.6.patch

Logged In: YES 
user_id=75003
Originator: NO

File Added: fcopy-async-tcl8.6.patch

File Added - 272962: fcopy-async-tcl8.5.patch

Logged In: YES 
user_id=75003
Originator: NO

File Added: fcopy-async-tcl8.5.patch

Logged In: YES 
user_id=75003
Originator: NO

File Added: fcopy-async-tcl8.4.patch

File Added - 272961: fcopy-async-tcl8.4.patch

File Added - 272954: fcopy-async.patch

Logged In: YES 
user_id=496139
Originator: YES

Fixed.

The attached patch fixes the issue of unwanted synchronous part.
The error suppression in the sync case was due to the fact that the code always called TclBackgroundException, regardless of the potentially synchronous status. This makes me think that the synchronous part was not intentional.
The bgerror works now. I mistakenly thought there was also error suppression in that case, dur to yet anothe bug not yet entered (the strange lappend-like concatenation of arguments to the callback, preventing callbacks like {puts FOO;error BAR}).

The patch is against vanilla code (without the -command patch), and includes the -command patch.
If you apply it against patched version, prepare for and ignore a failed hunk on the cmdPtr test.

Test suite OK.
File Added: fcopy-async.patch