Menu
▾
▴
#2351 Tcl_AsyncMark not delivered in tight loops
closed-fixed
5
2003-11-17
2003-05-31
No
If a bytecoded code sequence contains a tight loop such as
while {1} {}
(in other words, the loop contains no TCL_INVOKE* bytecodes
when compiled), then async events requested in the current
interpreter are not delivered.
Attached patch contains two test cases, one that works, and
one that goes into an endless loop.
Discussion
Logged In: YES
user_id=148712
The tests in the patch pass with the current HEAD - i.e.,
they do not expose the bug.
Logged In: YES
user_id=99768
Test cases revised to try to improve failure mode - was an infinite loop
New test cases
Logged In: YES
user_id=79902
Would a suitable fix be to add a new bytecode whose sole job
is to call the appropriate Tcl_Async* stuff? Then we could
modify the compiler to insert this new opcode when
appropriate (either in every loop - most easily done IMO -
or if we detect that the loop body contains no opcodes that
trigger checking for async events - the minimal requirement.)
Logged In: YES
user_id=148712
Option (0): check for async tasks at the top of the TEBC
loop, after every instruction. Easiest, definitely overkill,
probably unacceptable performance loss
Option (1): special bytecode for "command start" or "command
end". Easiest, performance? Can skip it for INST_EVAL,
INST_INVOKE and INST_EXPR as they do the right thing already.
Option (2): special bytecode emitted in every loop (and also
after every, say, fifth command)
Option (3): special bytecode emitted in every tight loop.
Best, tougher to implement as the compiler has to study the
"tightness" of the loops. A suitable definition could be: a
loop is tight if it has none of the previously mentioned
instructions. Also requires a solution for long non-loop bcc
sequences
Option (1) is the cleanest/easiest in my opinion. Never got
around to code it and time it
Logged In: YES
user_id=148712
Only in loops does not solve all issues; consider
proc runLong args \
[string repeat "set x [string repeat a 2000]\n" 9876543]
runLong
No async will be delivered while runLong is running, even if
we fix the loops. This argues for option 1, I guess
Logged In: YES
user_id=79902
Noting that this issue effectively has to be solved for
TIP#143 to be implementable.
Logged In: YES
user_id=148712
Attached a patch that checks for pending asyncs every 16th
instruction; the patch includes the tests of asynctest.patch.
Note that the tests require a threaded build to run.
Please review.
Logged In: YES
user_id=79902
Applied with a few extra tweaks. Is the updated patch a
candidate for 8.4?
async-4.2 and 4.3 fail on Linux. No idea why.
Patch already applied to HEAD
Logged In: YES
user_id=148712
Two small errors fixed in head incorporated to the patch.
Logged In: YES
user_id=79902
Reopening; still need assessment of whether this is a
candidate for 8.4
Logged In: YES
user_id=148712
Jeff: incorporate to 8.4 branch? This should really be a
safe patch, but it has not been stress tested. Also kinda
late ...
Logged In: YES
user_id=72656
I wimped out and decided not to make these changes for
8.4.5. It spanned a few changes, which indicates it had it's
little subtleties. For reference, the changes when in from
[1.114 to 1.117), in case we want to backport for 8.4.6.