#2939 Ttk async proc resets result of a command in main script

[Yevgen Ryazanov]

I found that in the middle of execution the result of [info library] was empty. To localize the problem, I called info library many times and at some call the result was empty, then back to normal. After setting a watch point on object result, I found that Ttk's procedure ThemeChangedProc called Tcl_GlobalEval without preserving Tcl result. Here's an example of stack where the result of [info library] is nullified:

[1] libtcl.so:ResetObjResult(iPtr = 0x85b03b8), line 968 in "tclResult.c"
[2] libtcl.so:Tcl_ResetResult(interp = (nil)), line 896 in "tclResult.c"
[3] libtcl.so:TclEvalEx(interp = 0x85b03b8, script = 0xf697895c "ttk::ThemeChanged", numBytes = 17, flags = 0, line = 1), line 4089 in "tclBasic.c"
[4] libtcl.so:Tcl_EvalEx(interp = 0x85b03b8, script = 0xf697895c "ttk::ThemeChanged", numBytes = -1, flags = 0), line 4043 in "tclBasic.c"
[5] libtcl.so:Tcl_Eval(interp = 0x85b03b8, script = 0xf697895c "ttk::ThemeChanged"), line 4823 in "tclBasic.c"
[6] libtcl.so:Tcl_GlobalEval(interp = 0x85b03b8, command = 0xf697895c "ttk::ThemeChanged"), line 5836 in "tclBasic.c"
[7] libtk.so:ThemeChangedProc(clientData = 0x860e818), line 519 in "ttkTheme.c"
[8] libtcl.so:TclServiceIdle(), line 738 in "tclTimer.c"
[9] libtcl.so:Tcl_DoOneEvent(flags = 34), line 991 in "tclNotify.c"
[10] libtk.so:MapFrame(clientData = 0x85d2c80), line 1751 in "tkFrame.c"
[11] libtcl.so:TclServiceIdle(), line 738 in "tclTimer.c"
[12] libtcl.so:Tcl_DoOneEvent(flags = -1), line 991 in "tclNotify.c"
[13] libeclutil.so:EventHandlerProc( = (nil), interp = 0x85b03b8, code = 0), line 367 in "gnm.C"
[14] libtcl.so:Tcl_AsyncInvoke(interp = 0x85b03b8, code = 0), line 240 in "tclAsync.c"
[15] libtcl.so:TclEvalObjvInternal(interp = 0x85b03b8, objc = 1, objv = 0x86723e0, command = (nil), length = 0, flags = 524288), line 3697 in "tclBasic.c"
[16] libtcl.so:Tcl_EvalObjv(interp = 0x85b03b8, objc = 1, objv = 0x86723e0, flags = 524288), line 3886 in "tclBasic.c"
[17] libtcl.so:NsEnsembleImplementationCmd(clientData = 0x85bf0e0, interp = 0x85b03b8, objc = 2, objv = 0x86722d8), line 6233 in "tclNamesp.c"
[18] libtcl.so:TclEvalObjvInternal(interp = 0x85b03b8, objc = 2, objv = 0x86722d8, command = 0x8088722 "info library] / $tcl_library"", length = 12, flags = 0), line 3690 in "tclBasic.c"
[19] libtcl.so:TclEvalEx(interp = 0x85b03b8, script = 0x8088722 "info library] / $tcl_library"", numBytes = 12, flags = 0, line = 1), line 4338 in "tclBasic.c"
[20] libtcl.so:TclSubstTokens(interp = 0x85b03b8, tokenPtr = 0x8671ee8, count = 4, tokensLeftPtr = (nil), line = 1), line 2254 in "tclParse.c"
[21] libtcl.so:TclEvalEx(interp = 0x85b03b8, script = 0x8088714 "puts "lib4 = [info library] / $tcl_library"", numBytes = 43, flags = 0, line = 1), line 4232 in "tclBasic.c"
[22] libtcl.so:Tcl_EvalEx(interp = 0x85b03b8, script = 0x8088714 "puts "lib4 = [info library] / $tcl_library"", numBytes = -1, flags = 0), line 4043 in "tclBasic.c"
[23] libtcl.so:Tcl_Eval(interp = 0x85b03b8, script = 0x8088714 "puts "lib4 = [info library] / $tcl_library""), line 4823 in "tclBasic.c"

I checked the latest Tk sources, 8.6b2, and the code of the function is the same.
After I added Tcl_SaveResult and Tcl_RestoreResult to the function, the problem is gone.
This change fixes the problem for me:

% cleartool diff -diff -pre ttkTheme.c
518c518,519
<
---
> Tcl_SavedResult state;
> Tcl_SaveResult (pkgPtr->interp, &state);
521a523
> Tcl_RestoreResult (pkgPtr->interp, &state);

You may want to preserve the whole state, not only result.
Good luck!

[Donal K. Fellows]

Unified diff

[Donal K. Fellows]

Seems like a reasonable analysis and the right general sort of fix. Elevating priority.

I've converted the suggested change to a patch that uses the Tcl 8.5 Tcl_InterpState API (which deals with more tricky cases than the Tcl_SavedResult API does) and attached it.

(For future reference, unified diffs are hugely easier to review as they include some context. No idea if clearcase can generate them though.)

[Joe English]

Tcl_GlobalEval("ttk::themeChanged" is called from within an idle handler, not as an async proc, so Tcl_{Save/Restore}InterpState is not necessary here.

From the stack backtrace, it looks like the cause of the problem is here: (see line [13]):

[11] libtcl.so:TclServiceIdle(), line 738 in "tclTimer.c"
[12] libtcl.so:Tcl_DoOneEvent(flags = -1), line 991 in "tclNotify.c"
[13] libeclutil.so:EventHandlerProc( = (nil), interp = 0x85b03b8, code = 0), line 367 in "gnm.C" <== HERE <==
[14] libtcl.so:Tcl_AsyncInvoke(interp = 0x85b03b8, code = 0), line 240 in "tclAsync.c"
[15] libtcl.so:TclEvalObjvInternal(interp = 0x85b03b8, objc = 1, objv = 0x86723e0, command = (nil), length = 0, flags = 524288), line 3697 in "tclBasic.c"
[16] libtcl.so:Tcl_EvalObjv(interp = 0x85b03b8, objc = 1, objv = 0x86723e0, flags = 524288), line 3886 in "tclBasic.c"
[17] libtcl.so:NsEnsembleImplementationCmd(clientData = 0x85bf0e0, interp = 0x85b03b8, objc = 2, objv = 0x86722d8), line 6233 in

It looks like you have a Tcl_AsyncProc that's reentering the event loop. (This is a very dangerous thing to do -- it can lead to all sorts of weird reentrancy bugs).

Anyway, that's the place where Tcl_{Preserve|Release}InterpState should go, in EventHandlerProc. (See the WARNING section in Tcl_AsyncInvoke(3)). Note that there are many other places in Tcl and Tk that use a Tcl_Interp* from idle handlers and event handlers, and almost none of them preserve the interp state either.

libeclutil.so:gnm.C(EventHandlerProc) isn't part of Tcl or Tk; lowering priority.

[Joe English]

(problem is in application code, not Tk; closing).

[Joe English]

Reopening per request from original reporter:

> Your conclusion about the reason of the problem is not right,
> but I got no chance to argue because the bug is closed.
> Tcl_DoOneEvent function is interp-free function. I have
> several interpreters in my code. I cannot save states of
> all existing interpreters as it follows from your suggestion.
> I have save/restore state in my function from the stack but
> only around the code that uses interp and not when I call
> Tcl_DoOneEvent because I don't think it is right thing to do.
> ThemeChangedProc uses interp saved as a global var (client data
> or other means) so it is its responsibility to save the state.
> Please review that code and let's fix the problem correctly.
>
> If you do not reopen the bug, I have to submit another one,
> I have asked a developer from my group to create a pure Tcl/Tk
> example which demonstrates the problem but I would prefer to
> skip this painful step.

[Joe English]

The cause of the problem is that the application is entering the event loop (Tcl_DoOneEvent) from within an async handler. This is not a good idea in the first place. It is the async handler's responsibility to save and restore the interpreter state (see the WARNING section in Tcl_AsyncInvoke manpage). This is not a bug in Tk.

[Yevgen Ryazanov]

Joe, after research and review I have to admit:
even though I may be right "theoretically", in practice I am wrong and you are completely right.
Async callbacks are available only in C code, so the problem is not possible in pure Tcl and my tries were waste. In C Async callback, calling update is a very poor idea.
Thank you for opportunity to discuss it.
Eugene

[Joe English]

(looks like it's safe to close this now, no further comments).