Tcl package Thread source code

View Ticket
Login
Ticket UUID: c6057948e5f884435ee35517008db0205f286a5e
Title: tsv commands break thread access rules for Tcl values
Type: Bug Version:
Submitter: dgp Created on: 2018-03-29 23:02:20
Subsystem: 80. Thread Package Assigned To: aku
Priority: 5 Medium Severity: Critical
Status: Open Last Modified: 2018-04-03 12:45:07
Resolution: None Closed By: nobody
    Closed on:
Description:
Multi-thread access to a Tcl_Obj struct is at best perilous
because the Tcl_IncrRefCount and Tcl_DecrRefCount public
routines are not routines at all, but macros.

And they are macros written with the strong assumption that
while they operate the caller has exclusive access to the field.
The only safe approach is to follow a strict rule that any
Tcl_Obj is accessed by only a single thread that can be said
to "own" that value. If you need to pass a value from one thread
to another, you need to have the second thread create its own
copy of the first, quite possibly limited only to the mechanism
of making a full copy of the string rep.

These rules are checked by the Tcl core when the --enable-symbols=mem
configuration setting is on. Tcl_Obj allocation stores the allocated
pointer in a thread-local hash map, and deallocation removes from
the same thread-local hash map.  Some mismatches (but not all?!?!)
are detected with a Tcl_Panic().
User Comments: dgp added on 2018-04-03 12:45:07:
There's a far bigger concern about general sharing of
a Tcl_Obj struct by multiple threads, though, and it
is the volatility of the internal representation. Shimmering.

Any code that interacts with an internal rep commonly has
a structure of using objPtr->typePtr to determine the routines
to use to act on the internal rep. Those routines proceed on
the belief the internal rep's nature is not going to change
while the routine runs.  All internal rep access and manipulation
is written on a foundational assumption of single-thread access.

If you get multiple threads passing through changing intrep details
out from under each other, there is going to be serious trouble.

dgp added on 2018-04-03 12:38:03:
The history of the objThreadMap restriction starts with
these checkins by Mo Dejong. First he tried adding a field
to the Tcl_Obj struct:

https://core.tcl.tk/tcl/info/50c5d9ac11c96d30

And then followed up with the objThreadMap approach:

https://core.tcl.tk/tcl/info/4dc4a16832209a53

The checkin and ChangeLog comments indicate the worry was
about races between multiple threads manipulating the
refCount field.  This is a valid concern, but with mutex
care and sufficient thread isolation it can be managed
without imposing such a strong constraint.

sebres added on 2018-04-03 08:56:04:
> without any regard to whether the code is running in the same thread that created that Tcl_Obj.

Why it is expected? The access to the objects in the tsv-storage is mutex-protected, and each set as well as the result of any tsv-call should always contain a safe duplication of the object (thus safe for any thread).

BTW I cannot reproduce the segfault with your example.

dgp added on 2018-03-30 11:50:17:
In tclUtil.c, there's a collection of code supporting ProcessGlobalValues,
which is one scheme for values to be shared among multiple threads.
Maybe it is useful as inspiration or as a foundation? Maybe some of its
routines would be useful to export?

dgp added on 2018-03-29 23:08:52:
The root of the problem is that the command procedures of
many tsv commands contain

    Tcl_DecrRefCount(svObj->tclObj);

without any regard to whether the code is running in the
same thread that created that Tcl_Obj.

Near as I can tell this entire sub-package is built on a misuse of the core.

dgp added on 2018-03-29 23:04:16:
With that background information, and using a --enable-symbols=mem
build of Tcl, run this demo script:

package require Thread

proc myScript mainTid {
    package require Thread
    thread::send -async $mainTid [list puts "ThreadRunning = [tsv::get sharedArray ThreadRunning]"]
    tsv::set sharedArray ThreadRunning 1
    thread::send -async $mainTid [list puts "ThreadRunning = [tsv::get sharedArray ThreadRunning]"]
}

proc installProc {tid procName} {
    set procBody [info body $procName]
    set procArgs [info args $procName]
    thread::send $tid [list proc $procName $procArgs $procBody]
}

tsv::set sharedArray ThreadRunning 0
set _threadHandle [thread::create]
installProc $_threadHandle myScript
thread::send -async $_threadHandle [list myScript [thread::id]]

after 1000

puts "\t Completed tests"


With any luck, one of those Panic safeguards will be triggered.