Обсуждение: Python 2.7 deprecated the PyCObject API?
According to a discussion over in Fedora-land, $subject is true: http://lists.fedoraproject.org/pipermail/devel/2010-August/140995.html I see several calls in plpython.c that seem to refer to PyCObject stuff. Anybody have any idea if we need to do something about this? regards, tom lane
On Aug 13, 2010, at 5:20 PM, Tom Lane wrote: > According to a discussion over in Fedora-land, $subject is true: > http://lists.fedoraproject.org/pipermail/devel/2010-August/140995.html > > I see several calls in plpython.c that seem to refer to PyCObject stuff. > Anybody have any idea if we need to do something about this? Well, we should at least be checking for an exception here anyways: proc->me = PyCObject_FromVoidPtr(proc, NULL); PyDict_SetItemString(PLy_procedure_cache, key, proc->me); if (proc->me == NULL) complain(); That is, with those warnings adjustments, proc->me will be NULL and then explode in PyDict_SetItemString: [David Malcolm] However, if someone overrides the process-wide warnings settings, then the API can fail altogether, raising a PendingDeprecationWarning exception (which in CPython terms means setting a thread-specific error state and returning NULL). ./ AFA a better fix is concerned, the shortest route would seem to be to use the new capsule stuff iff Python >= 2.7.
James William Pye <lists@jwp.name> writes:
> On Aug 13, 2010, at 5:20 PM, Tom Lane wrote:
>> I see several calls in plpython.c that seem to refer to PyCObject stuff.
>> Anybody have any idea if we need to do something about this?
> Well, we should at least be checking for an exception here anyways:
> proc->me = PyCObject_FromVoidPtr(proc, NULL);
> PyDict_SetItemString(PLy_procedure_cache, key, proc->me);
> if (proc->me == NULL) complain();
Just to clarify, you're recommending something like
proc->me = PyCObject_FromVoidPtr(proc, NULL);
+ if (proc->me == NULL)
+ elog(ERROR, "could not create PyCObject for function"); PyDict_SetItemString(PLy_procedure_cache, key,
proc->me);
correct? (Hm, and it looks like we'd better move the pfree just above that...)
> AFA a better fix is concerned, the shortest route would seem to be to
> use the new capsule stuff iff Python >= 2.7.
Yeah, and since we'll have to back-patch it, a fairly noninvasive patch
would be nice. Will you work on that?
regards, tom lane
On Aug 14, 2010, at 9:08 AM, Tom Lane wrote: > Just to clarify, you're recommending something like > > proc->me = PyCObject_FromVoidPtr(proc, NULL); > + if (proc->me == NULL) > + elog(ERROR, "could not create PyCObject for function"); > PyDict_SetItemString(PLy_procedure_cache, key, proc->me); > > correct? (Hm, and it looks like we'd better move the pfree just above that...) Almost, there's still a Python exception to report and/or clear. I only glanced at this and didn't recall what the plpython mechanisms were for that, thus the ambiguous "complain()". > Yeah, and since we'll have to back-patch it, a fairly noninvasive patch > would be nice. Will you work on that? I was hoping that Peter would pop in with a patch, but I think a few lines of CPP may suffice.. (warning: untested =) #ifdef Py_CAPSULE_H /** Python.h (2.7 and up) includes pycapsule.h, so rely on the header* define to detect the API's existence.*/ #define PyCObject_FromVoidPtr(POINTER, IGNORED) PyCapsule_New(POINTER, NULL, NULL) #undef PyCObject_Check #define PyCObject_Check(OBJ) PyCapsule_CheckExact(OBJ) #define PyCObject_AsVoidPtr(OBJ) PyCapsule_GetPointer(OBJ, NULL) #endif /* Py_CAPSULE_H */ http://svn.python.org/view/python/branches/release27-maint/Include/pycapsule.h?view=markup yay? nay?
James William Pye <lists@jwp.name> writes:
> On Aug 14, 2010, at 9:08 AM, Tom Lane wrote:
>> Just to clarify, you're recommending something like
>>
>> proc->me = PyCObject_FromVoidPtr(proc, NULL);
>> + if (proc->me == NULL)
>> + elog(ERROR, "could not create PyCObject for function");
>> PyDict_SetItemString(PLy_procedure_cache, key, proc->me);
>>
>> correct? (Hm, and it looks like we'd better move the pfree just above that...)
> Almost, there's still a Python exception to report and/or clear.
Ah, right, I guess that should be PLy_elog() not just elog().
>> Yeah, and since we'll have to back-patch it, a fairly noninvasive patch
>> would be nice. Will you work on that?
> I was hoping that Peter would pop in with a patch, but I think a few lines of CPP may suffice..
> ...
> yay? nay?
Damifino, I don't hack Python. Peter?
regards, tom lane
On fre, 2010-08-13 at 20:20 -0400, Tom Lane wrote: > According to a discussion over in Fedora-land, $subject is true: > http://lists.fedoraproject.org/pipermail/devel/2010-August/140995.html > > I see several calls in plpython.c that seem to refer to PyCObject > stuff. > Anybody have any idea if we need to do something about this? Some exception handling might be good, but I think we don't need to abandon the API yet. It's only "pending deprecation".
On tis, 2010-08-17 at 20:55 +0300, Peter Eisentraut wrote: > On fre, 2010-08-13 at 20:20 -0400, Tom Lane wrote: > > According to a discussion over in Fedora-land, $subject is true: > > http://lists.fedoraproject.org/pipermail/devel/2010-August/140995.html > > > > I see several calls in plpython.c that seem to refer to PyCObject > > stuff. > > Anybody have any idea if we need to do something about this? > > Some exception handling might be good, but I think we don't need to > abandon the API yet. It's only "pending deprecation". Here is a patch. The crash is reproducible, if warnings are turned into errors.
Вложения
On tis, 2010-08-17 at 21:48 +0300, Peter Eisentraut wrote: > On tis, 2010-08-17 at 20:55 +0300, Peter Eisentraut wrote: > > On fre, 2010-08-13 at 20:20 -0400, Tom Lane wrote: > > > According to a discussion over in Fedora-land, $subject is true: > > > http://lists.fedoraproject.org/pipermail/devel/2010-August/140995.html > > > > > > I see several calls in plpython.c that seem to refer to PyCObject > > > stuff. > > > Anybody have any idea if we need to do something about this? > > > > Some exception handling might be good, but I think we don't need to > > abandon the API yet. It's only "pending deprecation". > > Here is a patch. The crash is reproducible, if warnings are turned into > errors. I have applied this patch back to 8.0. 7.4's plpython crashes with Python >= 2.5; it's probably not worth rescuing.