Repository: cloudflare/stpyv8 (language: python)

Issue:
Memory leak
I have a python program that repeatedly queries a JS object that returns a JSON object.
Then I transform the object into a Python `dict` as follows:
```python3
import json
json_js = ctx.eval("JSON")
my_obj_js = ctx.eval("new MyClass()")

def query():
    return json.loads(json_js.stringify(my_obj_js.query()))
```

This creates the following memory leak crash after multiple queries (~100K).
```
<--- Last few GCs --->

[1475783:0x55e049377380]   455427 ms: Scavenge (reduce) 1468.0 (1496.8) -> 1467.1 (1497.0) MB, 3.1 / 0.0 ms  (average mu = 0.132, current mu = 0.031) allocation failure; 
[1475783:0x55e049377380]   455473 ms: Scavenge (reduce) 1468.3 (1497.0) -> 1467.3 (1497.3) MB, 2.6 / 0.0 ms  (average mu = 0.132, current mu = 0.031) allocation failure; 


<--- JS stacktrace --->


#
# Fatal javascript OOM in Ineffective mark-compacts near heap limit
#
```

This happens even if I restart the js-context every few calls.

When I change my program to "stringify" the JSON object in the JS implementation of the class, then the memory leak is resolved.
```python3
def query():
    return json.loads(my_obj_js.jsonQuery())
```
where `my_obj_js.jsonQuery()` returns `JSON.stringify(this.query())`.

From the STPyV8 implementation, I see that when a string is returned, then it is copied into a Python string.
But when an object is returned, it is using a C++ shared pointer.
My guess is that Python does not decrease the pointer when the object is released.

### Runtime
```
$ python3 --version
Python 3.10.6
$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 22.04.1 LTS
Release:	22.04
Codename:	jammy
```
With `stpyv8-ubuntu-22.04-python-3.10.zip`.


Discussion / hints:

Fix patch:
diff --git a/.github/workflows/build-debian-debug.yml b/.github/workflows/build-debian-debug.yml
index eaacbe83..1736c9e8 100644
--- a/.github/workflows/build-debian-debug.yml
+++ b/.github/workflows/build-debian-debug.yml
@@ -42,6 +42,8 @@ jobs:
       - name: STEP 6. Test wheel
         run: |
           pytest -v
+        env:
+          STPYV8_DEBUG: 1
 
       - name: STEP 7. Create wheel zip
         uses: vimtor/action-zip@v1.1
diff --git a/.github/workflows/build-osx-debug.yml b/.github/workflows/build-osx-debug.yml
index 4ad9e063..0cde04f7 100644
--- a/.github/workflows/build-osx-debug.yml
+++ b/.github/workflows/build-osx-debug.yml
@@ -44,6 +44,8 @@ jobs:
       - name: STEP 5. Test wheel
         run: |
           pytest -v
+        env:
+          STPYV8_DEBUG: 1
 
       - name: STEP 6. Create wheel zip
         uses: vimtor/action-zip@v1.1
diff --git a/.github/workflows/build-ubuntu-debug.yml b/.github/workflows/build-ubuntu-debug.yml
index d7faf99c..5ae53805 100644
--- a/.github/workflows/build-ubuntu-debug.yml
+++ b/.github/workflows/build-ubuntu-debug.yml
@@ -56,6 +56,8 @@ jobs:
       - name: STEP 5. Test wheel
         run: |
           pytest -v
+        env:
+          STPYV8_DEBUG: 1
 
       - name: STEP 6. Create wheel zip
         uses: vimtor/action-zip@v1.1
diff --git a/.github/workflows/release-debian-debug.yml b/.github/workflows/release-debian-debug.yml
index 9c4b0b66..bb090dd6 100644
--- a/.github/workflows/release-debian-debug.yml
+++ b/.github/workflows/release-debian-debug.yml
@@ -46,6 +46,8 @@ jobs:
       - name: STEP 6. Test wheel
         run: |
           pytest -v
+        env:
+          STPYV8_DEBUG: 1
 
       - name: STEP 7. Create wheel zip
         uses: vimtor/action-zip@v1.1
diff --git a/.github/workflows/release-debian.yml b/.github/workflows/release-debian.yml
index 07b3a901..89771c19 100644
--- a/.github/workflows/release-debian.yml
+++ b/.github/workflows/release-debian.yml
@@ -53,8 +53,8 @@ jobs:
           recursive: false
           dest: stpyv8-debian-11-python-${{ matrix.python-version }}.zip
 
-      - name: STEP 8. Upload to release
-        uses: AButler/upload-release-assets@v2.0
+      - name: STEP 8. Release
+        uses: softprops/action-gh-release@v1
         with:
           files: 'stpyv8-debian-11-python-${{ matrix.python-version }}.zip'
-          repo-token: ${{ secrets.GITHUB_TOKEN }}
+          token: ${{ secrets.GITHUB_TOKEN }}
diff --git a/.github/workflows/release-osx-debug.yml b/.github/workflows/release-osx-debug.yml
index 63d3cf5a..16bbbb5f 100644
--- a/.github/workflows/release-osx-debug.yml
+++ b/.github/workflows/release-osx-debug.yml
@@ -48,6 +48,8 @@ jobs:
       - name: STEP 5. Test wheel
         run: |
           pytest -v
+        env:
+          STPYV8_DEBUG: 1
 
       - name: STEP 6. Create wheel zip
         uses: vimtor/action-zip@v1.1
@@ -56,8 +58,8 @@ jobs:
           recursive: false
           dest: stpyv8-${{ matrix.os }}-python-${{ matrix.python-version }}-debug.zip
 
-      - name: STEP 7. Upload to release
-        uses: AButler/upload-release-assets@v2.0
+      - name: STEP 7. Release
+        uses: softprops/action-gh-release@v1
         with:
           files: 'stpyv8-${{ matrix.os }}-python-${{ matrix.python-version }}-debug.zip'
-          repo-token: ${{ secrets.GITHUB_TOKEN }}
+          token: ${{ secrets.GITHUB_TOKEN }}
diff --git a/.github/workflows/release-osx.yml b/.github/workflows/release-osx.yml
index 80c2750e..556d22a2 100644
--- a/.github/workflows/release-osx.yml
+++ b/.github/workflows/release-osx.yml
@@ -54,8 +54,8 @@ jobs:
           recursive: false
           dest: stpyv8-${{ matrix.os }}-python-${{ matrix.python-version }}.zip
 
-      - name: STEP 7. Upload to release
-        uses: AButler/upload-release-assets@v2.0
+      - name: STEP 7. Release
+        uses: softprops/action-gh-release@v1
         with:
           files: 'stpyv8-${{ matrix.os }}-python-${{ matrix.python-version }}.zip'
-          repo-token: ${{ secrets.GITHUB_TOKEN }}
+          token: ${{ secrets.GITHUB_TOKEN }}
diff --git a/.github/workflows/release-ubuntu-debug.yml b/.github/workflows/release-ubuntu-debug.yml
index c0b60279..4841ec42 100644
--- a/.github/workflows/release-ubuntu-debug.yml
+++ b/.github/workflows/release-ubuntu-debug.yml
@@ -60,6 +60,8 @@ jobs:
       - name: STEP 5. Test wheel
         run: |
           pytest -v
+        env:
+          STPYV8_DEBUG: 1
 
       - name: STEP 6. Create wheel zip
         uses: vimtor/action-zip@v1.1
@@ -68,8 +70,8 @@ jobs:
           recursive: false
           dest: stpyv8-${{ matrix.os }}-python-${{ matrix.python-version }}-debug.zip
 
-      - name: STEP 7. Upload to release
-        uses: AButler/upload-release-assets@v2.0
+      - name: STEP 7. Release
+        uses: softprops/action-gh-release@v1
         with:
           files: 'stpyv8-${{ matrix.os }}-python-${{ matrix.python-version }}-debug.zip'
-          repo-token: ${{ secrets.GITHUB_TOKEN }}
+          token: ${{ secrets.GITHUB_TOKEN }}
diff --git a/.github/workflows/release-ubuntu.yml b/.github/workflows/release-ubuntu.yml
index e01118be..34c63e2b 100644
--- a/.github/workflows/release-ubuntu.yml
+++ b/.github/workflows/release-ubuntu.yml
@@ -67,8 +67,8 @@ jobs:
           recursive: false
           dest: stpyv8-${{ matrix.os }}-python-${{ matrix.python-version }}.zip
 
-      - name: STEP 7. Upload to release
-        uses: AButler/upload-release-assets@v2.0
+      - name: STEP 7. Release
+        uses: softprops/action-gh-release@v1
         with:
           files: 'stpyv8-${{ matrix.os }}-python-${{ matrix.python-version }}.zip'
-          repo-token: ${{ secrets.GITHUB_TOKEN }}
+          token: ${{ secrets.GITHUB_TOKEN }}
diff --git a/.pylintrc b/.pylintrc
index 6043cded..a669d0ea 100644
--- a/.pylintrc
+++ b/.pylintrc
@@ -60,99 +60,28 @@ confidence=
 # --enable=similarities". If you want to run only the classes checker, but have
 # no Warning level messages displayed, use "--disable=all --enable=classes
 # --disable=W".
-disable=print-statement,
-        parameter-unpacking,
-        unpacking-in-except,
-        old-raise-syntax,
-        backtick,
-        long-suffix,
-        old-ne-operator,
-        old-octal-literal,
-        import-star-module-level,
-        non-ascii-bytes-literal,
-        raw-checker-failed,
-        bad-inline-option,
-        locally-disabled,
-        file-ignored,
-        suppressed-message,
-        useless-suppression,
+disable=bad-inline-option,
+        c-extension-no-member,
         deprecated-pragma,
-        use-symbolic-message-instead,
-        apply-builtin,
-        basestring-builtin,
-        buffer-builtin,
-        cmp-builtin,
-        coerce-builtin,
-        execfile-builtin,
-        file-builtin,
-        long-builtin,
-        raw_input-builtin,
-        reduce-builtin,
-        standarderror-builtin,
-        unicode-builtin,
-        xrange-builtin,
-        coerce-method,
-        delslice-method,
-        getslice-method,
-        setslice-method,
-        no-absolute-import,
-        old-division,
-        dict-iter-method,
-        dict-view-method,
-        next-method-called,
-        metaclass-assignment,
-        indexing-exception,
-        raising-string,
-        reload-builtin,
-        oct-method,
-        hex-method,
-        nonzero-method,
-        cmp-method,
-        input-builtin,
-        round-builtin,
-        intern-builtin,
-        unichr-builtin,
-        map-builtin-not-iterating,
-        zip-builtin-not-iterating,
-        range-builtin-not-iterating,
-        filter-builtin-not-iterating,
-        using-cmp-argument,
-        eq-without-hash,
-        div-method,
-        idiv-method,
-        rdiv-method,
-        exception-message-attribute,
-        invalid-str-codec,
-        sys-max-int,
-        bad-python3-import,
-        deprecated-string-function,
-        deprecated-str-translate-call,
-        deprecated-itertools-function,
-        deprecated-types-field,
-        next-method-defined,
-        dict-items-not-iterating,
-        dict-keys-not-iterating,
-        dict-values-not-iterating,
-        deprecated-operator-function,
-        deprecated-urllib-function,
-        xreadlines-attribute,
-        deprecated-sys-function,
-        exception-escape,
-        comprehension-escape,
+        duplicate-code,
+        file-ignored,
+        invalid-name,
+        locally-disabled,
+        missing-class-docstring,
         missing-function-docstring,
         missing-module-docstring,
-        missing-class-docstring,
-        invalid-name,
-        duplicate-code,
-        too-many-instance-attributes,
-        too-many-public-methods,
-        too-many-branches,
+        raw-checker-failed,
+        suppressed-message,
+        too-few-public-methods,
         too-many-arguments,
+        too-many-branches,
+        too-many-instance-attributes,
+        too-many-lines,
         too-many-locals,
+        too-many-public-methods,
         too-many-statements,
-        too-many-lines,
-        too-few-public-methods,
-        c-extension-no-member
+        use-symbolic-message-instead,
+        useless-suppression
 
 # Enable the message, report, category or checker with the given id(s). You can
 # either give multiple identifier separated by comma (,) or put this option
diff --git a/settings.py b/settings.py
index 3621ebc1..9884e270 100644
--- a/settings.py
+++ b/settings.py
@@ -7,7 +7,7 @@
 V8_HOME     = os.environ.get('V8_HOME', os.path.join(STPYV8_HOME, 'v8'))
 
 V8_GIT_URL        = "https://chromium.googlesource.com/v8/v8.git"
-V8_GIT_TAG_STABLE = "10.9.194.10"
+V8_GIT_TAG_STABLE = "11.0.226.13"
 V8_GIT_TAG_MASTER = "master"
 V8_GIT_TAG        = V8_GIT_TAG_STABLE
 DEPOT_GIT_URL     = "https://chromium.googlesource.com/chromium/tools/depot_tools.git"
diff --git a/src/Isolate.cpp b/src/Isolate.cpp
index c950b226..27685232 100644
--- a/src/Isolate.cpp
+++ b/src/Isolate.cpp
@@ -8,9 +8,12 @@
 size_t CIsolate::NearHeapLimitCallback(void *data,
                                        size_t current_heap_limit, size_t initial_heap_limit)
 {
+    v8::Isolate *isolate = (v8::Isolate *)data;
+
     if (current_heap_limit - initial_heap_limit > heap_max_increase)
         return current_heap_limit;
 
+    isolate->AdjustAmountOfExternalAllocatedMemory(heap_increase);
     return current_heap_limit + heap_increase;
 }
 
diff --git a/src/Wrapper.cpp b/src/Wrapper.cpp
index 6b85de81..8e4c14b6 100644
--- a/src/Wrapper.cpp
+++ b/src/Wrapper.cpp
@@ -857,9 +857,21 @@ v8::Handle<v8::Value> CPythonObject::WrapInternal(py::object obj)
         }
 
 #ifdef SUPPORT_TRACE_LIFECYCLE
-        py::object *object = new py::object(obj);
+        static v8::Persistent<v8::ObjectTemplate> s_template(isolate, CreateObjectTemplate(isolate));
 
-        ObjectTracer::Trace(jsobj.Object(), object);
+        v8::MaybeLocal<v8::Object> instance = v8::Local<v8::ObjectTemplate>::New(
+                isolate, s_template)->NewInstance(
+                        isolate->GetCurrentContext());
+
+        if (!instance.IsEmpty())
+        {
+            py::object *object = new py::object(obj);
+
+            v8::Handle<v8::Object> realInstance = instance.ToLocalChecked();
+            realInstance->SetInternalField(0, v8::External::New(isolate, object));
+
+            ObjectTracer::Trace(instance.ToLocalChecked(), object);
+        }
 #endif
 
         return handle_scope.Escape(jsobj.Object());
@@ -1841,7 +1853,7 @@ ObjectTracer::~ObjectTracer()
 
 void ObjectTracer::Dispose(void)
 {
-    // m_handle.ClearWeak();
+    m_handle.ClearWeak();
     m_handle.Reset();
 }
 
@@ -1856,19 +1868,18 @@ ObjectTracer& ObjectTracer::Trace(v8::Handle<v8::Value> handle, py::object *obje
 
 void ObjectTracer::Trace(void)
 {
-    // m_handle.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
+    m_handle.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
 
     m_living->insert(std::make_pair(m_object->ptr(), this));
 }
 
-/*
+
 void ObjectTracer::WeakCallback(const v8::WeakCallbackInfo<ObjectTracer>& info)
 {
-  std::unique_ptr<ObjectTracer> tracer(info.GetParameter());
+    CPythonGIL python_gil;
 
-  // assert(info.GetValue() == tracer->m_handle);
+    std::unique_ptr<ObjectTracer> tracer(info.GetParameter());
 }
-*/
 
 LivingMap * ObjectTracer::GetLivingMapping(void)
 {


Test patch:
diff --git a/tests/test_Engine.py b/tests/test_Engine.py
index 95eac6cb..ab772dbf 100644
--- a/tests/test_Engine.py
+++ b/tests/test_Engine.py
@@ -9,7 +9,7 @@
 class TestEngine(unittest.TestCase):
     def testClassProperties(self):
         with STPyV8.JSContext():
-            self.assertTrue(str(STPyV8.JSEngine.version).startswith("10."))
+            self.assertTrue(str(STPyV8.JSEngine.version).startswith("11."))
             self.assertFalse(STPyV8.JSEngine.dead)
 
     def testCompile(self):
diff --git a/tests/test_Wrapper.py b/tests/test_Wrapper.py
index b5cd1777..3424488f 100644
--- a/tests/test_Wrapper.py
+++ b/tests/test_Wrapper.py
@@ -5,9 +5,12 @@
 import os
 import datetime
 import unittest
+import pytest
 
 import STPyV8
 
+STPYV8_DEBUG = os.getenv("STPYV8_DEBUG") is not None
+
 
 def convert(obj):
     if isinstance(obj, STPyV8.JSArray):
@@ -608,7 +611,7 @@ def __init__(self):
             self.assertEqual(2, ctxt.eval("""array[1]"""))
 
     def testForEach(self):
-        class NamedClass(object):
+        class NamedClass:
             foo = 1 # pylint:disable=disallowed-name
 
             def __init__(self):
@@ -723,13 +726,13 @@ class Global(STPyV8.JSClass):
                 var none = null;
             """)
 
-            self.assertEqual(count+1, sys.getrefcount(None))
+            self.assertTrue(count <= sys.getrefcount(None) <= count + 1)
 
             ctxt.eval("""
                 var none = null;
             """)
 
-            self.assertEqual(count+1, sys.getrefcount(None))
+            self.assertTrue(count <= sys.getrefcount(None) <= count + 1)
 
             del ctxt
 
@@ -834,6 +837,18 @@ class Global(STPyV8.JSClass):
             self.assertTrue(ctxt.eval("b == b"))
             self.assertTrue(ctxt.eval("o == o"))
 
+    @pytest.mark.skipif(STPYV8_DEBUG, reason = "Not a test for debug mode")
+    def testMemoryLeak(self):
+        with STPyV8.JSIsolate():
+            with STPyV8.JSContext() as ctxt:
+                inner = ctxt.eval("i => Array(1<<20).fill(i)")
+                outer = ctxt.eval("o => o[0]")
+
+                for i in range(1000):
+                    outer(inner(i))
+
+                self.assertEqual(999, i)
+
     def testNamedSetter(self):
         class Obj(STPyV8.JSClass):
             @property