RE: [Python-Dev] Preventing 1.5 extensions crashing under 1.6/2.0 Python

Mark's comment about what I'd come up with being too complex inspired this simpler solution. Change the init function name to a new name PythonExtensionInit_ say. Pass in the API version for the extension writer to check. If the version is bad for this extension returns without calling any python functions. Add a return code that is true if compatible, false if not. If compatible the extension can use python functions and report and problems it wishes. int PythonExtensionInit_XXX( int invoking_python_api_version ) { if( invoking_python_api_version != PYTHON_API_VERSION ) { /* python will report that the module is incompatible */ return 0; } /* setup module for XXX ... */ /* say this extension is compatible with the invoking python */ return 1; } All 1.5 extensions fail to load on later python 2.0 and later. All 2.0 extensions fail to load on python 1.5. All new extensions work only with python of the same API version. Document that failure to setup a module could mean the extension is incompatible with this version of python. Small code change in python core. But need to tell extension writers what the new interface is and update all extensions within the python CVS tree. Barry

If someone in the core of Python thinks a patch implementing what I've outlined is useful please let me know and I will generate the patch. Barry

Umm - I'm afraid that I dont keep my python-dev emils for that long, and right now I'm too lazy/busy to dig around the archives. Exactly what did you outline? I know it went around a few times, and I can't remember who said what. For my money, I liked Fredrik's solution best (check Py_IsInitialized() in Py_InitModule4()), but as mentioned that only solves for the next version of Python; it doesnt solve the fact 1.5 modules will crash under 1.6/2.0 It would definately be excellent to get _something_ in the CNRI 1.6 release, so the BeOpen 2.0 release can see the results. But-I-doubt-anyone-will-release-extension-modules-for-1.6-anyway ly, Mark.

This is not a good way to solve the problem as it only works in a limited number of cases. Attached is my proposal which works for all new and old python and all old and new extensions.
It would definately be excellent to get _something_ in the CNRI 1.6 release, so the BeOpen 2.0 release can see the results.
But-I-doubt-anyone-will-release-extension-modules-for-1.6-anyway ly,
Yes indeed once the story of 1.6 and 2.0 is out I expect folks will skip 1.6. For example, if your win32 stuff is not ported then Python 1.6 is not usable on Windows/NT.
Mark.
Barry

I expect to be releasing a 1.6 Windows installer -- but I can't control Mark Hammond. Yet, it shouldn't be hard for him to create a 1.6 version of win32all, should it?
I sort-of like this idea -- at least at the +0 level. I would choose a shorter name: PyExtInit_XXX(). Could you (or someone else) prepare a patch that changes this? It would be great if the patch were relative to the 1.6 branch of the source tree; unfortunately this is different because of the ANSIfication. Unfortunately we only have two days to get this done for 1.6 -- I plan to release 1.6b1 this Friday! If you don't get to it, prepare a patch for 2.0 would be the next best thing. --Guido van Rossum (home page: http://www.pythonlabs.com/~guido/)

"not usable"? guess you haven't done much cross-platform development lately...
huh? are you seriously proposing to break every single C extension ever written -- on each and every platform -- just to trap an error message caused by extensions linked against 1.5.2 on your favourite platform?
you mean "update the source code for all extensions ever written." -1

True. On Unix I have an ISDN status monitor, it depends on FReeBSD interfaces and PIL. On Windows I have an SCM solution that depends on COM to drive SourceSafe. Without Mark's COM support I cannot run any of my code on Windows.
What makes you think that a crash will not happen under Unix when you change the API? You just don't get the Windows crash. As this thread has pointed out you have no intention of checking for binary compatibility on the API as you move up versions.
Yes, I'm aware of the impact.

[/F]
[Barry]
I imtimated the following, but did not spell it out, so I will here to clarify. I was -1 on Barry's solution getting into 1.6, given the time frame. I hinted that the solution Guido recently checked in "if (!Py_IsInitialized()) ..." would not be too great an impact even if Barry's solution, or one like it, was eventually adopted. So I think that the adoption of our half-solution (ie, we are really only forcing a better error message - not even getting a traceback to indicate _which_ module fails) need not preclude a better solution when we have more time to implement it... Mark.

mark wrote:
note that the module name is available to the Py_InitModule4 module (for obvious reasons ;-), so it's not that difficult to improve the error message. how about: ... static char not_initialized_error[] = "ERROR: Module %.200s loaded an uninitialized interpreter!\n\ This Python has API version %d, module %.200s has version %d.\n"; ... if (!Py_IsInitialized()) { char message[500]; sprintf(message, not_initialized_error, name, PYTHON_API_VERSION, name, module_api_version) Py_FatalError(message); } </F>

Guido van Rossum wrote:
I sort of dislike the idea ;-) It introduces needless work for hundreds of extension writers and effectively prevents binary compatibility for future versions of Python: not all platforms have the problems of the Windows platform and extensions which were compiled against a different API version may very well still work with the new Python version -- e.g. the dynamic loader on Linux is very well capable of linking the new Python version against an extension compiled for the previous Python version. If all this is really necessary, I'd at least suggest adding macros emulating the old Py_InitModule() APIs, so that extension writers don't have to edit their code just to get it recompiled. BTW, the subject line doesn't have anything to do with the proposed solutions in this thread... they all crash Python or the extensions in some way, some nicer, some not so nice ;-) -- Marc-Andre Lemburg ______________________________________________________________________ Business: http://www.lemburg.com/ Python Pages: http://www.lemburg.com/python/

[Re forcing all extensions to use PythonExtensionInit_XXX]
I sort-of like this idea -- at least at the +0 level.
Since this email there have been some strong objections to this. I too would weigh in at -1 for this, simply for the amount of work it would cost me personally!
It is now Friday afternoon for me. Regardless of the outcome of this, the patch Fredrik posted recently would still seem reasonable, and not have too much impact on performance (ie, after locating and loading a .dll/.so, one function call isnt too bad!): I've even left his trailing comment, which I agree with too? Shall this be checked in to the 1.6 and 2.0 trees? Mark. Index: Python/modsupport.c =================================================================== RCS file: /cvsroot/python/python/dist/src/Python/modsupport.c,v retrieving revision 2.48 diff -u -r2.48 modsupport.c --- Python/modsupport.c 2000/07/09 03:09:56 2.48 +++ Python/modsupport.c 2000/07/18 07:55:03 @@ -51,6 +51,8 @@ { PyObject *m, *d, *v; PyMethodDef *ml; + if (!Py_IsInitialized()) + Py_FatalError("Interpreter not initialized (version mismatch?)"); if (module_api_version != PYTHON_API_VERSION) fprintf(stderr, api_version_warning, name, PYTHON_API_VERSION, name, module_api_version); "Fatal Python error: Interpreter not initialized" might not be too helpful, but it's surely better than "PyThreadState_Get: no current thread"...

On Fri, Aug 04, 2000 at 03:58:52PM +1000, Mark Hammond wrote:
+ if (!Py_IsInitialized()) + Py_FatalError("Interpreter not initialized (version
Wasn't there a problem with this, because the 'Py_FatalError()' would be the one in the uninitialized library and thus result in the same tstate error ? Perhaps it needs a separate error message, that avoids the usual Python cleanup and trickery and just prints the error message and exits ? -- Thomas Wouters <thomas@xs4all.net> Hi! I'm a .signature virus! copy me into your .signature file to help me spread!

I would obviously need to test this, but a cursory look at Py_FatalError() implies it does not touch the thread lock - simply an fprintf, and an abort() (and for debug builds on Windows, an offer to break into the debugger) Regardless, I'm looking for a comment on the concept, and I will make sure that whatever I do actually works ;-) Mark.

thomas wrote:
you mean this one: Py_FatalError("PyThreadState_Get: no current thread");
Perhaps it needs a separate error message, that avoids the usual Python cleanup and trickery and just prints the error message and exits ?
void Py_FatalError(char *msg) { fprintf(stderr, "Fatal Python error: %s\n", msg); #ifdef macintosh for (;;); #endif #ifdef MS_WIN32 OutputDebugString("Fatal Python error: "); OutputDebugString(msg); OutputDebugString("\n"); #ifdef _DEBUG DebugBreak(); #endif #endif /* MS_WIN32 */ abort(); } </F>

[Re forcing all extensions to use PythonExtensionInit_XXX]
[GvR]
I sort-of like this idea -- at least at the +0 level.
[MH]
OK. Dead it is. -1.
Shall this be checked in to the 1.6 and 2.0 trees?
Yes, I'll do so.
"Fatal Python error: Interpreter not initialized" might not be too helpful, but it's surely better than "PyThreadState_Get: no current thread"...
Yes. --Guido van Rossum (home page: http://www.pythonlabs.com/~guido/)

If someone in the core of Python thinks a patch implementing what I've outlined is useful please let me know and I will generate the patch. Barry

Umm - I'm afraid that I dont keep my python-dev emils for that long, and right now I'm too lazy/busy to dig around the archives. Exactly what did you outline? I know it went around a few times, and I can't remember who said what. For my money, I liked Fredrik's solution best (check Py_IsInitialized() in Py_InitModule4()), but as mentioned that only solves for the next version of Python; it doesnt solve the fact 1.5 modules will crash under 1.6/2.0 It would definately be excellent to get _something_ in the CNRI 1.6 release, so the BeOpen 2.0 release can see the results. But-I-doubt-anyone-will-release-extension-modules-for-1.6-anyway ly, Mark.

This is not a good way to solve the problem as it only works in a limited number of cases. Attached is my proposal which works for all new and old python and all old and new extensions.
It would definately be excellent to get _something_ in the CNRI 1.6 release, so the BeOpen 2.0 release can see the results.
But-I-doubt-anyone-will-release-extension-modules-for-1.6-anyway ly,
Yes indeed once the story of 1.6 and 2.0 is out I expect folks will skip 1.6. For example, if your win32 stuff is not ported then Python 1.6 is not usable on Windows/NT.
Mark.
Barry

I expect to be releasing a 1.6 Windows installer -- but I can't control Mark Hammond. Yet, it shouldn't be hard for him to create a 1.6 version of win32all, should it?
I sort-of like this idea -- at least at the +0 level. I would choose a shorter name: PyExtInit_XXX(). Could you (or someone else) prepare a patch that changes this? It would be great if the patch were relative to the 1.6 branch of the source tree; unfortunately this is different because of the ANSIfication. Unfortunately we only have two days to get this done for 1.6 -- I plan to release 1.6b1 this Friday! If you don't get to it, prepare a patch for 2.0 would be the next best thing. --Guido van Rossum (home page: http://www.pythonlabs.com/~guido/)

"not usable"? guess you haven't done much cross-platform development lately...
huh? are you seriously proposing to break every single C extension ever written -- on each and every platform -- just to trap an error message caused by extensions linked against 1.5.2 on your favourite platform?
you mean "update the source code for all extensions ever written." -1

True. On Unix I have an ISDN status monitor, it depends on FReeBSD interfaces and PIL. On Windows I have an SCM solution that depends on COM to drive SourceSafe. Without Mark's COM support I cannot run any of my code on Windows.
What makes you think that a crash will not happen under Unix when you change the API? You just don't get the Windows crash. As this thread has pointed out you have no intention of checking for binary compatibility on the API as you move up versions.
Yes, I'm aware of the impact.

[/F]
[Barry]
I imtimated the following, but did not spell it out, so I will here to clarify. I was -1 on Barry's solution getting into 1.6, given the time frame. I hinted that the solution Guido recently checked in "if (!Py_IsInitialized()) ..." would not be too great an impact even if Barry's solution, or one like it, was eventually adopted. So I think that the adoption of our half-solution (ie, we are really only forcing a better error message - not even getting a traceback to indicate _which_ module fails) need not preclude a better solution when we have more time to implement it... Mark.

mark wrote:
note that the module name is available to the Py_InitModule4 module (for obvious reasons ;-), so it's not that difficult to improve the error message. how about: ... static char not_initialized_error[] = "ERROR: Module %.200s loaded an uninitialized interpreter!\n\ This Python has API version %d, module %.200s has version %d.\n"; ... if (!Py_IsInitialized()) { char message[500]; sprintf(message, not_initialized_error, name, PYTHON_API_VERSION, name, module_api_version) Py_FatalError(message); } </F>

Guido van Rossum wrote:
I sort of dislike the idea ;-) It introduces needless work for hundreds of extension writers and effectively prevents binary compatibility for future versions of Python: not all platforms have the problems of the Windows platform and extensions which were compiled against a different API version may very well still work with the new Python version -- e.g. the dynamic loader on Linux is very well capable of linking the new Python version against an extension compiled for the previous Python version. If all this is really necessary, I'd at least suggest adding macros emulating the old Py_InitModule() APIs, so that extension writers don't have to edit their code just to get it recompiled. BTW, the subject line doesn't have anything to do with the proposed solutions in this thread... they all crash Python or the extensions in some way, some nicer, some not so nice ;-) -- Marc-Andre Lemburg ______________________________________________________________________ Business: http://www.lemburg.com/ Python Pages: http://www.lemburg.com/python/

[Re forcing all extensions to use PythonExtensionInit_XXX]
I sort-of like this idea -- at least at the +0 level.
Since this email there have been some strong objections to this. I too would weigh in at -1 for this, simply for the amount of work it would cost me personally!
It is now Friday afternoon for me. Regardless of the outcome of this, the patch Fredrik posted recently would still seem reasonable, and not have too much impact on performance (ie, after locating and loading a .dll/.so, one function call isnt too bad!): I've even left his trailing comment, which I agree with too? Shall this be checked in to the 1.6 and 2.0 trees? Mark. Index: Python/modsupport.c =================================================================== RCS file: /cvsroot/python/python/dist/src/Python/modsupport.c,v retrieving revision 2.48 diff -u -r2.48 modsupport.c --- Python/modsupport.c 2000/07/09 03:09:56 2.48 +++ Python/modsupport.c 2000/07/18 07:55:03 @@ -51,6 +51,8 @@ { PyObject *m, *d, *v; PyMethodDef *ml; + if (!Py_IsInitialized()) + Py_FatalError("Interpreter not initialized (version mismatch?)"); if (module_api_version != PYTHON_API_VERSION) fprintf(stderr, api_version_warning, name, PYTHON_API_VERSION, name, module_api_version); "Fatal Python error: Interpreter not initialized" might not be too helpful, but it's surely better than "PyThreadState_Get: no current thread"...

On Fri, Aug 04, 2000 at 03:58:52PM +1000, Mark Hammond wrote:
+ if (!Py_IsInitialized()) + Py_FatalError("Interpreter not initialized (version
Wasn't there a problem with this, because the 'Py_FatalError()' would be the one in the uninitialized library and thus result in the same tstate error ? Perhaps it needs a separate error message, that avoids the usual Python cleanup and trickery and just prints the error message and exits ? -- Thomas Wouters <thomas@xs4all.net> Hi! I'm a .signature virus! copy me into your .signature file to help me spread!

I would obviously need to test this, but a cursory look at Py_FatalError() implies it does not touch the thread lock - simply an fprintf, and an abort() (and for debug builds on Windows, an offer to break into the debugger) Regardless, I'm looking for a comment on the concept, and I will make sure that whatever I do actually works ;-) Mark.

thomas wrote:
you mean this one: Py_FatalError("PyThreadState_Get: no current thread");
Perhaps it needs a separate error message, that avoids the usual Python cleanup and trickery and just prints the error message and exits ?
void Py_FatalError(char *msg) { fprintf(stderr, "Fatal Python error: %s\n", msg); #ifdef macintosh for (;;); #endif #ifdef MS_WIN32 OutputDebugString("Fatal Python error: "); OutputDebugString(msg); OutputDebugString("\n"); #ifdef _DEBUG DebugBreak(); #endif #endif /* MS_WIN32 */ abort(); } </F>

[Re forcing all extensions to use PythonExtensionInit_XXX]
[GvR]
I sort-of like this idea -- at least at the +0 level.
[MH]
OK. Dead it is. -1.
Shall this be checked in to the 1.6 and 2.0 trees?
Yes, I'll do so.
"Fatal Python error: Interpreter not initialized" might not be too helpful, but it's surely better than "PyThreadState_Get: no current thread"...
Yes. --Guido van Rossum (home page: http://www.pythonlabs.com/~guido/)
participants (6)
-
Barry Scott
-
Fredrik Lundh
-
Guido van Rossum
-
M.-A. Lemburg
-
Mark Hammond
-
Thomas Wouters