[PATCH] rlm_python fixes
Please consider applying the following patch to rlm_python.c This fixes various issues (resulting in coredumps) with threads. I've found the modified source while debugging my own radius/python installation somewhere on the net. The result is very stable compared to original rlm_python - running for 4 month without coredump. -- Paul P 'Stingray' Komkoff Jr // http://stingr.net/key <- my pgp key This message represents the official view of the voices in my head
Paul P Komkoff Jr <i@stingr.net> wrote:
Please consider applying the following patch to rlm_python.c
Hmm... see bugs.freeradius.org for yet another patch to the python module. The main problem I have with your patch is that I can't tell what's changed. You've mixed up re-formatting, re-arrangement, and new code all in the same patch. If you could send a series of patches, each doing *one* thing, it would be a lot easier to tell what the code does. And that would allow us to separate what appears to be gratuitous re-arrangement from bug fixes. Alan DeKok.
Replying to Alan DeKok:
Paul P Komkoff Jr <i@stingr.net> wrote:
Please consider applying the following patch to rlm_python.c
Hmm... see bugs.freeradius.org for yet another patch to the python module.
http://bugs.freeradius.org/show_bug.cgi?id=227 This is exactly the same code I've sent you.
The main problem I have with your patch is that I can't tell what's changed. You've mixed up re-formatting, re-arrangement, and new code all in the same patch.
From comments on this bug you can see that patch author tried to modify module, but after some time he just rewrote it from scratch ...
If you could send a series of patches, each doing *one* thing, it would be a lot easier to tell what the code does.
And that would allow us to separate what appears to be gratuitous re-arrangement from bug fixes.
Well... it isn;t my rewrite, but indeed it fixes couple of bugs just because of being redone using "modern" (wrt current python documentation) style for embedding python. Nevermind. If it is _so_ required I'll try to comment the changes. Maybe will catch some bugs too. But current rlm_python is indeed unusable. :) -- Paul P 'Stingray' Komkoff Jr // http://stingr.net/key <- my pgp key This message represents the official view of the voices in my head
Replying to Alan DeKok:
The main problem I have with your patch is that I can't tell what's changed. You've mixed up re-formatting, re-arrangement, and new code all in the same patch.
Well... 1. Changed the way rlm_python_t is declared (more common to C applications) 2. Added WITH_THREAD_POOL and corresponding ifdefs, which wrap python_start_thread, destroy thread and some python thread support function calls - to allow threaded operation. (I think some of this code can be optimised away by using 2.3+ PyGILState_Ensure function but this can be marked as TODO after initial work will be accepted) 3. Added a lot of Py_XDECREFs and a smaller number of INCREFs where needed 4. Added error handling where it's marked as TODO in original module. -- Paul P 'Stingray' Komkoff Jr // http://stingr.net/key <- my pgp key This message represents the official view of the voices in my head
participants (2)
-
Alan DeKok -
Paul P Komkoff Jr