Tk Source Code

Test send-8.16 fails:

==== send-8.16 Tk_SendCmd procedure, bogusCommWindow FAILED
---- Result was:
1 {target application died}
---- Result should have been (exact matching):
1 {no application named "bogus"}

It failed intermittently before 8.6.16 (1-8 of 10 runs) and fails always since 8.6.16, including the current core-8-6-branch, core-9-0-branch and main. send appends the command to the property of the target's comm window in AppendPropCarefully(). If the target window does not exist, the BadWindow error is handled by AppendErrorProc(), which sets the result "no application named ..." and marks the command as complete. There are two bugs:

1. Race. If the error is read by Xlib during the XFlush() in TkUnixDoOneXEvent(), no event is queued and the socket is empty, so select() waits until the 2 second timeout and TkUnixDoOneXEvent() returns 0. The wait loop in Tk_SendCmd() does not check pending.gotResponse after a timeout: it calls ValidateName() and overwrites the result with "target application died" (leaking the previous result). If the error arrives a bit later, select() returns, and the test passes.
2. Since [1602306bfc] (fix for [f52986c698]), Tk_DeleteErrorHandler() sets errorProc to NULL, so the callback of a deleted handler is never called. But the manual (CrtErrHdlr.3) says that it may still be called for errors from requests made before deletion. AppendPropCarefully() deletes the handler right after XChangeProperty(), so the late BadWindow error is now always ignored, and send always waits 2 seconds and returns "target application died".

The manual (CrtErrHdlr.3) still describes the old behavior of Tk_DeleteErrorHandler(), and other code (e.g. extensions, since this is a public API) can depend on it.

Jan backported it to [dd98a8b50d|core-9-0-branch] (fixed a typo in [6410716225]). Backported to [4468e763f7|core-8-6-branch].

So, I cannot find anything wrong with this fix (on the contrary!)

Merged to [2957ed0e|trunk] now. Backporting to 9.0 and 8.6 is OK to me (whoever does it)

> so the fix for this ticket is not merged yet.

Indeed. Fixed the commit text now, pointing to the correct branches, not this one.

Right, tkUnixFont.c has no error handler with a callback.

Note that [db7cd2265c], which mentions send-late-x-errors, merged focus-test-xfocus, so the fix for this ticket is not merged yet.

> How about tkUnixFont.c? Should the same change be done there too?

I guess the answer is NO, I don't see an errorFlag there.

How about tkUnixFont.c? Should the same change be done there too?

Proposed fix in branch send-late-x-errors:

• Tk_DeleteErrorHandler() again behaves as documented. The crash in [f52986c698] was caused by the font code (since [7f20966df9], workaround for [3767882e06]), which passed the address of a local variable to an error handler; an error reported after the function returned wrote into the stack frame of another function. Now the flag is in thread-specific data.
• AppendPropCarefully() calls XSync() before deleting the error handler for synchronous sends, and the wait loop in Tk_SendCmd() no longer overwrites a response set while waiting.
• failsOnCILinux and failsOnXQuartz are removed from send-8.16. failsOnCILinux is also removed from send-8.15: it passes on all versions and platforms I tried, so it was probably added by mistake.