[Tcl-bugs] [ tcl-Bugs-1932639 ] Mixed sync/async [fcopy -command]

[SourceForge.net <no...@so...>]

Bugs item #1932639, was opened at 2008-04-02 22:49
Message generated for change (Comment added) made by ferrieux
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=1932639&group_id=10894

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: 24. Channel Commands
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Alexandre Ferrieux (ferrieux)
Assigned to: Andreas Kupries (andreas_kupries)
Summary: Mixed sync/async [fcopy -command]

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


----------------------------------------------------------------------

>Comment By: Alexandre Ferrieux (ferrieux)
Date: 2008-04-03 11:49

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

----------------------------------------------------------------------

Comment By: Andreas Kupries (andreas_kupries)
Date: 2008-04-03 01:37

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

----------------------------------------------------------------------

Comment By: Andreas Kupries (andreas_kupries)
Date: 2008-04-03 00:46

Message:
Logged In: YES 
user_id=75003
Originator: NO

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

----------------------------------------------------------------------

Comment By: Andreas Kupries (andreas_kupries)
Date: 2008-04-03 00:45

Message:
Logged In: YES 
user_id=75003
Originator: NO

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

----------------------------------------------------------------------

Comment By: Andreas Kupries (andreas_kupries)
Date: 2008-04-03 00:45

Message:
Logged In: YES 
user_id=75003
Originator: NO

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

----------------------------------------------------------------------

Comment By: Alexandre Ferrieux (ferrieux)
Date: 2008-04-03 00:02

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

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=1932639&group_id=10894