Thread: TCL fix in HEAD
The recent TCL patch assumed Tcl_NotifierProcs.initNotifierProc was
added in TCL 8.2:
#if HAVE_TCL_VERSION(8,2)
/*
* Override the functions in the Notifier subsystem. See comments above.
*/
{
Tcl_NotifierProcs notifier;
notifier.setTimerProc = pltcl_SetTimer;
notifier.waitForEventProc = pltcl_WaitForEvent;
notifier.createFileHandlerProc = pltcl_CreateFileHandler;
notifier.deleteFileHandlerProc = pltcl_DeleteFileHandler;
notifier.initNotifierProc = pltcl_InitNotifier;
In fact it was added in 8.4 so I have modified the CVS with the
following patch.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/pl/tcl/pltcl.c
===================================================================
RCS file: /cvsroot/pgsql/src/pl/tcl/pltcl.c,v
retrieving revision 1.113
diff -c -c -r1.113 pltcl.c
*** src/pl/tcl/pltcl.c 21 Sep 2007 00:30:49 -0000 1.113
--- src/pl/tcl/pltcl.c 28 Sep 2007 21:02:55 -0000
***************
*** 178,184 ****
* within Postgres, but we implement all the functions for completeness.
* We can only fix this with Tcl >= 8.2, when Tcl_SetNotifier() appeared.
*/
! #if HAVE_TCL_VERSION(8,2)
static ClientData
pltcl_InitNotifier(void)
--- 178,184 ----
* within Postgres, but we implement all the functions for completeness.
* We can only fix this with Tcl >= 8.2, when Tcl_SetNotifier() appeared.
*/
! #if HAVE_TCL_VERSION(8,4)
static ClientData
pltcl_InitNotifier(void)
***************
*** 262,268 ****
Tcl_FindExecutable("");
#endif
! #if HAVE_TCL_VERSION(8,2)
/*
* Override the functions in the Notifier subsystem. See comments above.
*/
--- 262,268 ----
Tcl_FindExecutable("");
#endif
! #if HAVE_TCL_VERSION(8,4)
/*
* Override the functions in the Notifier subsystem. See comments above.
*/
>The recent TCL patch assumed Tcl_NotifierProcs.initNotifierProc >was added in TCL 8.2: >In fact it was added in 8.4 so I have modified the CVS with >the following patch. I confirmed this against the 8.2.5 release. Sorry I did not notice that when I initially created the patch. While the patch will not work for the earlier releases, it should be noted that the multithhreading problem still exists when linking postgresql against TCL 8.2 and 8.3 libraries that are compiled with the preprocessor symbol TCL_THREADS defined. Unfortunately, we cannot override the initNotifier behavior in those releases, so we don't have a workable solution. This is probably not a big problem, since TCL was not commonly compiled with multithreading enabled prior to 8.4. However, perhaps there should be a warning in the documentation on PL/TCL directing users to avoid linking postgresql against TCL libraries earlier than 8.4 that have multithreading enabled? Yours, Steve Marshall
On Friday 28 September 2007 14:04, Bruce Momjian wrote: > ** 178,184 **** > * within Postgres, but we implement all the functions for completeness. > * We can only fix this with Tcl >= 8.2, when Tcl_SetNotifier() appeared. > */ > ! #if HAVE_TCL_VERSION(8,2) You missed the 8.2 in the comment. Matt
Matt Newell wrote: > On Friday 28 September 2007 14:04, Bruce Momjian wrote: > > ** 178,184 **** > > * within Postgres, but we implement all the functions for completeness. > > * We can only fix this with Tcl >= 8.2, when Tcl_SetNotifier() appeared. > > */ > > ! #if HAVE_TCL_VERSION(8,2) > > You missed the 8.2 in the comment. Thanks, fixed. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Marshall, Steve wrote: > >The recent TCL patch assumed Tcl_NotifierProcs.initNotifierProc > >was added in TCL 8.2: > >In fact it was added in 8.4 so I have modified the CVS with > >the following patch. > > I confirmed this against the 8.2.5 release. Sorry I did not notice that > when I initially created the patch. > > While the patch will not work for the earlier releases, it should be > noted that the multithhreading problem still exists when linking > postgresql against TCL 8.2 and 8.3 libraries that are compiled with the > preprocessor symbol TCL_THREADS defined. Unfortunately, we cannot > override the initNotifier behavior in those releases, so we don't have a > workable solution. > > This is probably not a big problem, since TCL was not commonly compiled > with multithreading enabled prior to 8.4. However, perhaps there should > be a warning in the documentation on PL/TCL directing users to avoid > linking postgresql against TCL libraries earlier than 8.4 that have > multithreading enabled? Can you send in a patch against pltcl.sgml? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Marshall, Steve wrote:
>> This is probably not a big problem, since TCL was not commonly compiled
>> with multithreading enabled prior to 8.4. However, perhaps there should
>> be a warning in the documentation on PL/TCL directing users to avoid
>> linking postgresql against TCL libraries earlier than 8.4 that have
>> multithreading enabled?
> Can you send in a patch against pltcl.sgml?
I'm not sure there's much point, as someone using a PG release new
enough to contain such a disclaimer is probably also using a reasonably
new Tcl. (Tcl 8.4 was released five years ago...)
regards, tom lane
Bruce Momjian wrote:
>Marshall, Steve wrote:
>
>
>>>The recent TCL patch assumed Tcl_NotifierProcs.initNotifierProc
>>>was added in TCL 8.2:
>>>In fact it was added in 8.4 so I have modified the CVS with
>>>the following patch.
>>>
>>>
>>I confirmed this against the 8.2.5 release. Sorry I did not notice that
>>when I initially created the patch.
>>
>>While the patch will not work for the earlier releases, it should be
>>noted that the multithhreading problem still exists when linking
>>postgresql against TCL 8.2 and 8.3 libraries that are compiled with the
>>preprocessor symbol TCL_THREADS defined. Unfortunately, we cannot
>>override the initNotifier behavior in those releases, so we don't have a
>>workable solution.
>>
>>This is probably not a big problem, since TCL was not commonly compiled
>>with multithreading enabled prior to 8.4. However, perhaps there should
>>be a warning in the documentation on PL/TCL directing users to avoid
>>linking postgresql against TCL libraries earlier than 8.4 that have
>>multithreading enabled?
>>
>>
>
>Can you send in a patch against pltcl.sgml?
>
>
PL/TCL documentation patch is attached.
*** pltcl.sgml 2007-10-01 08:29:06.667578247 -0400
--- pltcl.sgml.new 2007-10-01 08:24:41.736708719 -0400
***************
*** 70,75 ****
--- 70,87 ----
<literal>createlang pltcl <replaceable>dbname</></literal> or
<literal>createlang pltclu <replaceable>dbname</></literal>.
</para>
+ <para>
+ Care should be taken when linking the pltcl shared object code against
+ TCL libraries earlier than the TCL 8.4 release. The pre-8.4 versions of TCL must
+ be built <emphasis>without</emphasis> multithreading support, i.e. with TCL_THREADS
+ undefined. Otherwise, the first use of PL/TCL functions will cause the postgres
+ backend to become multithreaded, resulting in subsequent "unexpected state" errors.
+ PL/TCL <emphasis>can</emphasis> be safely linked against multithreaded versions of the
+ TCL library for TCL versions 8.4 and later. In these cases, the pltcl source code uses
+ capabilities introduced in TCL 8.4 to override and disable the multithreading behavior.
+ Note that pre-8.4 versions of TCL were rarely built with multithreading support in
+ pre-compiled distributions, so this problem is rather rare.
+ </para>
</sect1>
<!-- **** PL/Tcl description **** -->
"Marshall, Steve" <smarshall@wsi.com> writes:
> Bruce Momjian wrote:
>> Can you send in a patch against pltcl.sgml?
>>
> PL/TCL documentation patch is attached.
This seems a bit wordy, as well as wrongly placed --- the time to tell
people they need a non-multithreaded Tcl is in the installation
instructions. I added the following instead.
regards, tom lane
Index: installation.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/installation.sgml,v
retrieving revision 1.292
diff -c -r1.292 installation.sgml
*** installation.sgml 25 Aug 2007 20:29:25 -0000 1.292
--- installation.sgml 1 Oct 2007 16:42:50 -0000
***************
*** 228,234 ****
<listitem>
<para>
If you want to build the <application>PL/Tcl</application>
! procedural language, you of course need a Tcl installation.
</para>
</listitem>
--- 228,237 ----
<listitem>
<para>
If you want to build the <application>PL/Tcl</application>
! procedural language, you of course need a <productname>Tcl</>
! installation. If you are using a pre-8.4 release of
! <productname>Tcl</>, ensure that it was built without multithreading
! support.
</para>
</listitem>
I'm fine with Tom's wording and placement. >This seems a bit wordy, as well as wrongly placed --- the time to tell >people they need a non-multithreaded Tcl is in the installation >instructions. I added the following instead. > > regards, tom lane > > >