Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Python memory issues #1886

Merged
merged 3 commits into from
Oct 2, 2018

Conversation

matiasinsaurralde
Copy link
Contributor

Related to #1228.

Added Python refcount macros. Modified the C.free calls to avoid any GC issues.

Python_DispatchHook uses memcpy to avoid accessing the internal buffer of the resulting PyObject.

Added Python refcount macros. Modified the C.free calls to avoid any GC issues.

Python_DispatchHook uses memcpy to avoid accessing the internal buffer of the resulting PyObject.
@matiasinsaurralde
Copy link
Contributor Author

Tests are ok now!

@buger
Copy link
Member

buger commented Aug 28, 2018

@matiasinsaurralde I see a TODO statement in comments, are you going to do it in terms of this PR?

@buger buger merged commit c245af2 into TykTechnologies:master Oct 2, 2018
buger pushed a commit that referenced this pull request Oct 2, 2018
* coprocess: fix Python memory issues

Added Python refcount macros. Modified the C.free calls to avoid any GC issues.

Python_DispatchHook uses memcpy to avoid accessing the internal buffer of the resulting PyObject.

* coprocess: modify Lua code to match dispatcher arguments

* coprocess: add error results
buger pushed a commit that referenced this pull request Mar 1, 2020
…2895)

Fix for #2894, similar to #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 #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.
tykbot bot pushed a commit that referenced this pull request Mar 1, 2020
…2895)

Fix for #2894, similar to #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 #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.

(cherry picked from commit 0f8d881)
tykbot bot pushed a commit that referenced this pull request Mar 1, 2020
…2895)

Fix for #2894, similar to #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 #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.

(cherry picked from commit 0f8d881)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants