Skip to content

Commit

Permalink
coprocess_python: makes sure that PyBytesFromString releases memory. (T…
Browse files Browse the repository at this point in the history
…ykTechnologies#2895)

Fix for TykTechnologies#2894, similar to TykTechnologies#1886:

1. Implemented bindings for `Py_IncRef`, `Py_DecRef` and `PyTuple_ClearFreeList` (could be potentially used).
2. Corrected `free` call in `PyBytesFromString`.
3. Added refcount handlers in `coprocess_python.go`.

With this patch, GC object count looks better over time (using the middleware attached in TykTechnologies#2894):

```
myhook called
29509
myhook called
29509
myhook called
29509
myhook called
29509
myhook called
29509
myhook called
29509
myhook called
29509
myhook called
29509
myhook called
29509
myhook called
29509
myhook called
29509
myhook called
29509
myhook called
29509
myhook called
29509
```

Under some scenarios, additional tweaks might be required (either on the plugin side or Tyk Python code side) to run the GC collection more often, etc.
  • Loading branch information
matiasinsaurralde committed Mar 1, 2020
1 parent c420f0a commit 0f8d881
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 4 deletions.
36 changes: 36 additions & 0 deletions dlpython/binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,18 @@ PyObject* PyImport_Import(PyObject* name) { return PyImport_Import_ptr(name); };
typedef PyObject* (*PyObject_CallObject_f)(PyObject*, PyObject*);
PyObject_CallObject_f PyObject_CallObject_ptr;
PyObject* PyObject_CallObject(PyObject* callable_object, PyObject* args) { return PyObject_CallObject_ptr(callable_object, args); };
typedef void (*Py_IncRef_f)(PyObject*);
Py_IncRef_f Py_IncRef_ptr;
void Py_IncRef(PyObject* object) { return Py_IncRef_ptr(object); };
typedef void (*Py_DecRef_f)(PyObject*);
Py_DecRef_f Py_DecRef_ptr;
void Py_DecRef(PyObject* object) { return Py_DecRef_ptr(object); };
typedef int (*PyTuple_ClearFreeList_f)();
PyTuple_ClearFreeList_f PyTuple_ClearFreeList_ptr;
int PyTuple_ClearFreeList() { return PyTuple_ClearFreeList_ptr(); };
*/
import "C"
import (
Expand Down Expand Up @@ -179,6 +191,18 @@ func PyObject_CallObject(callable_object *C.PyObject, args *C.PyObject) *C.PyObj
return C.PyObject_CallObject(callable_object, args)
}

func Py_IncRef(object *C.PyObject) {
C.Py_IncRef(object)
}

func Py_DecRef(object *C.PyObject) {
C.Py_DecRef(object)
}

func PyTuple_ClearFreeList() C.int {
return C.PyTuple_ClearFreeList()
}

func mapCalls() error {
C.python_lib = C.dlopen(libPath, C.RTLD_NOW|C.RTLD_GLOBAL)

Expand Down Expand Up @@ -264,6 +288,18 @@ func mapCalls() error {
defer C.free(unsafe.Pointer(s_PyObject_CallObject))
C.PyObject_CallObject_ptr = C.PyObject_CallObject_f(C.dlsym(C.python_lib, s_PyObject_CallObject))

s_Py_IncRef := C.CString("Py_IncRef")
defer C.free(unsafe.Pointer(s_Py_IncRef))
C.Py_IncRef_ptr = C.Py_IncRef_f(C.dlsym(C.python_lib, s_Py_IncRef))

s_Py_DecRef := C.CString("Py_DecRef")
defer C.free(unsafe.Pointer(s_Py_DecRef))
C.Py_DecRef_ptr = C.Py_DecRef_f(C.dlsym(C.python_lib, s_Py_DecRef))

s_PyTuple_ClearFreeList := C.CString("PyTuple_ClearFreeList")
defer C.free(unsafe.Pointer(s_PyTuple_ClearFreeList))
C.PyTuple_ClearFreeList_ptr = C.PyTuple_ClearFreeList_f(C.dlsym(C.python_lib, s_PyTuple_ClearFreeList))

dlErr := C.dlerror()
if dlErr != nil {
// TODO: create a proper Go error from dlerror output
Expand Down
22 changes: 18 additions & 4 deletions dlpython/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func PyObjectGetAttr(o unsafe.Pointer, attr interface{}) (unsafe.Pointer, error)
// PyBytesFromString wraps PyBytesFromString
func PyBytesFromString(input []byte) (unsafe.Pointer, error) {
data := C.CBytes(input)
// defer C.free(unsafe.Pointer(data))
defer C.free(unsafe.Pointer(data))
ret := PyBytes_FromStringAndSize((*C.char)(data), C.long(len(input)))
if ret == nil {
return nil, errors.New("PyBytesFromString failed")
Expand All @@ -149,17 +149,31 @@ func PyBytesFromString(input []byte) (unsafe.Pointer, error) {

// PyBytesAsString wraps PyBytes_AsString
func PyBytesAsString(o unsafe.Pointer, l int) ([]byte, error) {
cstr := PyBytes_AsString(ToPyObject(o))
obj := ToPyObject(o)
cstr := PyBytes_AsString(obj)
if cstr == nil {
return nil, errors.New("PyBytes_AsString as string failed")
}
// defer C.free(unsafe.Pointer(cstr))
str := C.GoBytes(unsafe.Pointer(cstr), C.int(l))
return []byte(str), nil
b := []byte(str)
return b, nil
}

// PyLongAsLong wraps PyLong_AsLong
func PyLongAsLong(o unsafe.Pointer) int {
l := PyLong_AsLong(ToPyObject(o))
return int(l)
}

func PyIncRef(o unsafe.Pointer) {
Py_IncRef(ToPyObject(o))
}

func PyDecRef(o unsafe.Pointer) {
Py_DecRef(ToPyObject(o))
}

func PyTupleClearFreeList() int {
sz := PyTuple_ClearFreeList()
return int(sz)
}
3 changes: 3 additions & 0 deletions gateway/coprocess_python.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ func (d *PythonDispatcher) Dispatch(object *coprocess.Object) (*coprocess.Object
pythonLock.Unlock()
return nil, err
}
python.PyDecRef(args)
python.PyDecRef(dispatchHookFunc)

newObjectPtr, err := python.PyTupleGetItem(result, 0)
if err != nil {
Expand Down Expand Up @@ -116,6 +118,7 @@ func (d *PythonDispatcher) Dispatch(object *coprocess.Object) (*coprocess.Object
pythonLock.Unlock()
return nil, err
}
python.PyDecRef(result)
pythonLock.Unlock()

newObject := &coprocess.Object{}
Expand Down

0 comments on commit 0f8d881

Please sign in to comment.