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:
- 2803109.patch [download] added by dgp on 2009-06-09 11:23:00. [details]