Tk Source Code

View Ticket
Login
Ticket UUID: 1114977
Title: tkCanvas.c thread-unsafe UID usage!
Type: Bug Version: obsolete: 8.4.9
Submitter: nobody Created on: 2005-02-02 19:24:07
Subsystem: 04. Canvas Basics Assigned To: dkf
Priority: 9 Immediate Severity:
Status: Closed Last Modified: 2005-06-04 06:23:07
Resolution: Fixed Closed By: dkf
    Closed on: 2005-06-03 23:23:07
Description:
WHEN:
When multiple threads of execution in same process have
their own TCL vm's with canvas widgets instances.
WHAT:
canvas's "tags" functionality gets totally screwed. For
example, IWidget::calendar crashes
HOW:
tkCanvas.c contains definitions of various variables
for uid caching. For example: "static Tk_Uid allUid;"
(line 21). These variables get assigned their
respective values when first instance of canvas in
_whole_ _process_ (!) is being instantiated. This is
done in "Tk_CanvasObjCmd" by means of function
"CanvasInit" call (line 370). "allUid", and other
similar variables get their values as return values
from Tk_GetUid function call (line 2781).
All values that are to be compared with several special
words (like "all" is) are compared with said variables
(code like: if( Tk_GetUid( "something ) == allUid))
However, Tk_GetUid uses dictionary (hash table) which
gets allocated per-thread, not per-process. That means
that values of "allUid" and similar variables will get
initialized with keys from dictionary belonging to
first thread that ever creates canvas! All subsequently
created canvasses will use these values (from first
thread's UID dictionary). Suppose that canvas belonging
to other thread of execution wants to compare
"something" with such keys. It will use "Tk_Uid(
"something" ) == allUid" and this will always fall,
regardless of "something"'s value.
FIX:
Quick and dirty fix is to uncomment line "#define
USE_OLD_TAG_SEARCH 1" (line 19 in tkCanvas.c), since
old search algorithm uses no global data. This works,
althought I'm not aware of other consequences it may
have. Nice fix would be to change either these global
variables to per-thread (or per-instance) ones or make
Tk_GetUid use global dictionary. Alas, I don't know
enough about TCL/TK source code to make such changes on
my own. However, should someone experienced give me
advice on what to do, I'll try to find time to fix it.
Best regards
Michal Adamczak (ma<NOSPAM>@ant-iss.com)
User Comments: dkf added on 2005-06-04 06:23:07:
Logged In: YES 
user_id=79902

backported

dgp added on 2005-06-04 02:27:03:
Logged In: YES 
user_id=80530


hmmm..
backport slipped 8.4.10
priority already maxed.

hobbs added on 2005-02-05 09:12:07:
Logged In: YES 
user_id=72656

Great, if it left in the existing tag search behavior, I
think it should get backported to 8.4.

dkf added on 2005-02-04 17:19:19:

File Added - 118543: canvas_globals.diff

Logged In: YES 
user_id=79902

I attach the patch I used (which was sophisticated enough;
it works by either protecting static variables with a mutex
or transferring them into thread-local storage)

hobbs added on 2005-02-04 01:22:19:
Logged In: YES 
user_id=72656

The problem with the fix is that the "OLD_TAG_SEARCH"
doesn't support the advanced tag syntax, so a more
sophisticated fix is necessary.

dkf added on 2005-02-03 20:48:30:
Logged In: YES 
user_id=79902

Fixed in HEAD (change from tkCanvas.c 1.31->1.33)
Backport candidate?

Attachments: