Tcl Source Code

View Ticket
Login
Ticket UUID: 2803109
Title: tsv - string rep - core dump
Type: Bug Version: obsolete: 8.6b1.1
Submitter: juliannoble Created on: 2009-06-08 18:11:49
Subsystem: 10. Objects Assigned To: dgp
Priority: 5 Medium Severity:
Status: Closed Last Modified: 2009-06-10 21:44:56
Resolution: Fixed Closed By: dgp
    Closed on: 2009-06-10 14:44:56
Description:
Tcl 8.6b1.1 on FreeBSD - (multi core system)

Attempting to read back an empty string/empty list from a tsv  is causing the process to crash with:
UpdateStringProc for type 'string' failed to create a valid string rep

The initialisation is done with:
tsv::set t_1 accounts_on_thread [list]

Retrieving the value (in the same thread)  later  like so:
set streams_on_thisthread [tsv::get t_1 accounts_on_thread]

This assignment appears to work - but any subsequent attempt to use the value $streams_on_thisthread as a string causes the crash.
(although 'string length' works and returns 0, but 'string bytelength' triggers the crash)
Using the Tweezer package the type is reported as string and the refcount is 2.




Initialising it the following way also results in the problem:
tsv::set t_1 accounts_on_thread ""

The following values however work without error:
tsv::set t_1 accounts_on_thread " "
tsv::set t_1 accounts_on_thread [lrange [list] 0 0]
tsv::set t_1 accounts_on_thread [string range "" 0 -1]
tsv::set t_1 accounts_on_thread [string range "" 0 0]


I've been unable to reproduce the error in a simple script.
(the real app has a handful of threads and is doing some sqlite work - the crash occurs around 30s in)

(see also discussion on clt:  http://groups.google.com/group/comp.lang.tcl/browse_frm/thread/6fec903446cd87a4/4a8a163a0755856a#4a8a163a0755856a )
User Comments: dgp added on 2009-06-10 21:44:56:

allow_comments - 1

fix committed.

dgp added on 2009-06-10 01:17:45:
some more details on what was wrong, and
how the patch deals with it...

The "string" Tcl_ObjType failed to always
handle the case of a "pure unicode" empty
string.  In UpdateStringOfString() it failed
to create a string rep, leading to the panic
reported here.  In AppendUtfToUtf() it 
leaped right into appending without checking
that it had something in place to append to.

Elsewhere in the implementation of the
"string" Tcl_ObjType, these deficiencies
were dealt with by trying to prevent the
creation of "pure unicode" empty string
values.  Everywhere such a value would
normally be created, the usual empty
string rep was added so that the value was
no longer a "pure unicode" value.

The problem with that approach is that
the routine Tcl_InvalidateStringRep() is
public, and so long as the stork model
is respected, extensions are supposed to
be free to create "pure intrep" values.
That's what the Thread package was
doing.

What the patch does is first to fix up those
parts of the "string" ObjType implementation
that could not handle a "pure unicode" empty
string value.  Then, since that value is no
longer troublesome, other parts of the implementation
that tried to prevent the creation of such a value
have been removed.  So now the kind of 
value that triggered the panic is much more
routinely created with Tcl itself, with no need
for an extension to do it.  This means that
existing tests in the test suite already cover
the new code when the patch is applied.

dgp added on 2009-06-10 00:37:41:
The way to test this is with additions to
the teststringobj command.

andreas_kupries added on 2009-06-10 00:27:49:
Hm ... Actually, I do not remember.

ferrieux added on 2009-06-10 00:23:07:
Does the success of the testthread constraint (ie the presence of the testthread command IIRC) imply the presence of the Thread package in its full glory, including tsv::* ?

Side question: would it make sense to have [package require Thread] either in the constraint code or in the test itself, doing nothing if absent ?

andreas_kupries added on 2009-06-10 00:16:48:
Actually we have something like 'testthread' as test constraint. Look into the IO tests, for the reflected channels and transform. There I have basically two parallel test suites, for with and without threads.

ferrieux added on 2009-06-10 00:11:19:
Fixes my minimal example. Incurs no regression in the test suite. Good :)

(I assume this example ought to be turned into a test case, but I see no previous example of tests conditioned by the availability of an extension... so maybe it could wait until Thread is bundled ?)

dgp added on 2009-06-09 11:23:19:
Please test the attached patch.

dgp added on 2009-06-09 11:23:00:

File Added - 330268: 2803109.patch

ferrieux added on 2009-06-09 04:58:45:
OK I have a minimal repro, now that you have explained the mechanism on the chat:

 % package require Thread
 % set x ""
 % string length $x
 % tsv::set a b $x
 % puts [tsv::get a b]
 UpdateStringProc for type 'string' failed to create a valid string rep

dgp added on 2009-06-09 04:34:11:
This appears to be flaw in the revised
UpdateStringOfString.

andreas_kupries added on 2009-06-09 01:38:42:
Re-assigned to Zoran as the maintainer of the Thread extension.
Which seems to be heavily involved.

Attachments: