THRIFT-3765 fix memory leak in python compact protocol extension#970
THRIFT-3765 fix memory leak in python compact protocol extension#970ccmaymay wants to merge 1 commit intoapache:masterfrom
Conversation
|
@cjmay thanks for catching this before the release. |
| @@ -431,6 +433,7 @@ bool ProtocolBase<Impl>::encodeValue(PyObject* value, TType type, PyObject* type | |||
| } | |||
There was a problem hiding this comment.
I think we need DECREF before the "return false" above too.
Feel free to use ScopedPyObject (or not).
There was a problem hiding this comment.
I thought so too, originally, but isn't value null there?
There was a problem hiding this comment.
It shouldn't be...
The value is either null-checked or passed to Py_INCREF that does not accept null.
The intention here is to simply reject too long string values.
If you experienced null value here, we'll need to investigate on that matter too.
There was a problem hiding this comment.
Oh, I apologize, I was looking at the wrong "return false."
There was a problem hiding this comment.
Fixed, and thanks for the tip.
|
Thank you! We were excited to see this extension get written. |
|
Committed, thanks again for the fix. |
3 participants
This fixes a significant memory leak in the new Python extension module for the compact protocol.