The mutex arrangement in TclInitSubsystems() seems
to suffer from a significant flaw. Consider two threads,
A and B, calling the routine concurrently. Thread A gets
the lock first, sets subsystemsInitialized to 1 and starts
calling the set of subsystem initializing routines.
Meanwhile thread B makes the initial test
(subsystemsInitialized == 0) and since thread A
has already set it to 1, thread B jumps straight
to the bottom of TIS() and calls TclInitNotifier().
The trouble is when B calls TclInitNotifier(), we
have no idea how much of Tcl library initialization
is actually done. We just know it got started.
Branch bug-3611974 committed:
Revisions to the multi-thread arrangments of TclInitSubsystems().
With these changes, the caller of TclInitSubsystems() can be sure that
at some point after the TIS() call is begun and before it returned, the Tcl
library became fully initialized. Without these revisions, that cannot be
guaranteed, TIS() can return with Tcl library initialization having never
been finished.
Even with these revisions, the caller cannot be sure the library remains
initialized. Any thread might call Tcl_Finalize() at any time. That's a
deeper problem which this patch does not attempt to address.
Two remarks:
- Tcl_Finalize could be handled as well by using 4 states in stead of 3:
CHANGING -> STARTING | FINISHING
- If Tcl_CreateInterp wouldn't call TclInitSubsystems(), but in stead
the application was required to call TclInitSubsystems() first, this
problem wouldn't exist at all. But that's food for Tcl 9.
Further on, I agree with this change. It doesn't really solve
the problem, but a panic is better than leaving the system
in an undefined state. I'll have a further look.
Of course, once we make Tcl_InitSubsystems() a public
routine that any extension might call at any time (buggy
as that might be), the problem will come right back.
The 4 state makes no functional difference that I can see,
but I agree it feels a bit more polished that way. I'll
commit that change to the branch.
Thanks for looking it over.
Working through this got me started
thinking about ways to do this better, but
as you say, that's a novem project. Will
keep that in other branches and tickets.