Merge branch 'caleb/fix/this' into philippe/pyTypeFactory-cleanup

Author: philippedistributive

.github/workflows/test-and-publish.yaml

@@ -8,6 +8,17 @@ on:
       - '*'
   workflow_call:
   workflow_dispatch:
+    inputs:
+      debug_enabled:
+        type: boolean
+        description: 'Run the build with tmate debugging enabled (https://github.com/marketplace/actions/debugging-with-tmate)'
+        required: false
+        default: false
+      dump_cores:
+        type: boolean
+        description: 'Include core dumps in CI artifacts'
+        required: false
+        default: false
   pull_request:
 
 env:
@@ -31,13 +42,13 @@ jobs:
         python_version: [ '3.10' ]
     runs-on: ${{ matrix.os }}
     steps:
-      - uses: actions/checkout@v3
-      - uses: actions/setup-python@v4
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python_version }}
       - name: Cache spidermonkey build
         id: cache-spidermonkey
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: |
             ./_spidermonkey_install/*
@@ -54,10 +65,10 @@ jobs:
   build-spidermonkey-win:
     runs-on: windows-2019
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Cache spidermonkey build
         id: cache-spidermonkey
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: |
             ./_spidermonkey_install/*
@@ -104,12 +115,12 @@ jobs:
             python_version: '3.9'
     runs-on: ${{ matrix.os }}
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
         with:
           submodules: recursive
           fetch-depth: 0 # fetch all history for all branches and tags
           # poetry-dynamic-versioning needs git tags to produce the correct version number 
-      - uses: actions/setup-python@v4
+      - uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python_version }}
       - name: Setup Poetry
@@ -138,7 +149,7 @@ jobs:
           poetry install --no-root --only=dev
           echo "Installed Dependencies"
       - name: Use cached spidermonkey build
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: |
             ./_spidermonkey_install/*
@@ -168,13 +179,16 @@ jobs:
           name: docs-${{ github.run_id }}-${{ github.sha }}
           path: ./build/docs/html/
       - name: Set cores to get stored in /cores
+        if: ${{ matrix.os != 'windows-2019' && github.event_name == 'workflow_dispatch' && inputs.dump_cores }}
+        # TODO (Caleb Aikens) figure out how to get Windows core dumps
         run: |
-          if [[ "$OSTYPE" == "linux-gnu"* || "$OSTYPE" == "darwin"* ]]; then
-            # TODO (Caleb Aikens) figure out how to get Windows core dumps
-            sudo mkdir /cores
-            sudo chmod 777 /cores
-            # Core filenames will be of the form executable.pid.timestamp:
-            sudo bash -c 'echo "/cores/%e.%p.%t" > /proc/sys/kernel/core_pattern'
+          sudo mkdir -p /cores
+          sudo chmod 777 /cores
+          # Core filenames will be of the form osname.pythonversion.executable.pid.timestamp:
+          if [[ "$OSTYPE" == "linux-gnu"* ]]; then
+            sudo bash -c 'echo "/cores/${OSTYPE}.$(poetry run python --version).%e.%p.%t" > /proc/sys/kernel/core_pattern'
+          else
+            sudo sysctl kern.corefile="/cores/${OSTYPE}.$(poetry run python --version).%e.%p.%y"
           fi
       - name: Run Python tests (pytest)
         run: |
@@ -185,25 +199,34 @@ jobs:
           poetry run python -m pip install --force-reinstall --verbose ./dist/*
           poetry run python -m pytest tests/python
       - name: Run JS tests (peter-jr)
+        if: ${{ (success() || failure()) }}
         run: |
           if [[ "$OSTYPE" == "linux-gnu"* || "$OSTYPE" == "darwin"* ]]; then
           # TODO (Caleb Aikens) figure out how to get Windows core dumps
             ulimit -c unlimited
           fi
           poetry run bash ./peter-jr ./tests/js/
-      - uses: actions/upload-artifact@v3
-        if: ${{ failure() && matrix.os != 'windows-2019' }} # Run only if something went wrong
+      - name: Upload core dumps as CI artifacts
+        uses: actions/upload-artifact@v3
+        if: ${{ matrix.os != 'windows-2019' && github.event_name == 'workflow_dispatch' && inputs.dump_cores }}
         # TODO (Caleb Aikens) figure out how to get Windows core dumps
         with:
-          name: cores
+          name: cores-${{ matrix.os }}-${{ matrix.python_version }}
           path: /cores
+      # Enable tmate debugging of manually-triggered workflows if the input option was provided
+      - name: SSH debug session
+        if: ${{ (success() || failure()) && github.event_name == 'workflow_dispatch' && inputs.debug_enabled }}
+        uses: mxschmitt/action-tmate@v3
+        with:
+          detached: true
+          limit-access-to-actor: true
   sdist:
     runs-on: ubuntu-20.04
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
         with:
           fetch-depth: 0
-      - uses: actions/setup-python@v4
+      - uses: actions/setup-python@v5
         with:
           python-version: '3.9'
       - name: Setup Poetry
@@ -226,7 +249,7 @@ jobs:
     if: ${{ success() && github.event_name == 'push' && contains(github.ref, 'refs/tags/') }}
     steps:
       # no need to checkout
-      - uses: actions/setup-python@v4
+      - uses: actions/setup-python@v5
         with:
           python-version: '3.9'
       - run: pip install twine

CMakeLists.txt

@@ -61,6 +61,7 @@ if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)
     set(Python_FIND_VIRTUALENV FIRST) # (require cmake >= v3.15 and this is the default) use the Python version configured by pyenv if available
     set(PYTHON_LIBRARIES ${Python_LIBRARIES})
     set(PYTHON_INCLUDE_DIR ${Python_INCLUDE_DIRS})
+    message("Linux - Using Python:${Python_VERSION_MAJOR}.${Python_VERSION_MINOR} - Libraries:${PYTHON_LIBRARIES} - IncludeDirs: ${PYTHON_INCLUDE_DIR}")
     find_package(SpiderMonkey REQUIRED)
   elseif(WIN32)
     find_package(PythonInterp 3.8 REQUIRED)

include/PyEventLoop.hh

@@ -31,15 +31,21 @@ public:
    * @see https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.Handle
    */
   struct AsyncHandle {
+    using id_t = uint32_t;
+    using id_ptr_pair = std::pair<id_t, AsyncHandle *>;
   public:
     explicit AsyncHandle(PyObject *handle) : _handle(handle) {};
     AsyncHandle(const AsyncHandle &old) = delete; // forbid copy-initialization
-    AsyncHandle(AsyncHandle &&old) : _handle(std::exchange(old._handle, nullptr)) {}; // clear the moved-from object
+    AsyncHandle(AsyncHandle &&old) : _handle(std::exchange(old._handle, nullptr)), _refed(old._refed.exchange(false)) {}; // clear the moved-from object
     ~AsyncHandle() {
       if (Py_IsInitialized()) { // the Python runtime has already been finalized when `_timeoutIdMap` is cleared at exit
         Py_XDECREF(_handle);
       }
     }
+    static inline id_ptr_pair newEmpty() {
+      auto handle = AsyncHandle(Py_None);
+      return AsyncHandle::getUniqueIdAndPtr(std::move(handle));
+    }
 
     /**
      * @brief Cancel the scheduled event-loop job.
@@ -51,18 +57,23 @@ public:
      * @brief Get the unique `timeoutID` for JS `setTimeout`/`clearTimeout` methods
      * @see https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#return_value
      */
-    static inline uint32_t getUniqueId(AsyncHandle &&handle) {
+    static inline id_t getUniqueId(AsyncHandle &&handle) {
       // TODO (Tom Tang): mutex lock
       _timeoutIdMap.push_back(std::move(handle));
       return _timeoutIdMap.size() - 1; // the index in `_timeoutIdMap`
     }
-    static inline AsyncHandle *fromId(uint32_t timeoutID) {
+    static inline AsyncHandle *fromId(id_t timeoutID) {
       try {
         return &_timeoutIdMap.at(timeoutID);
       } catch (...) { // std::out_of_range&
         return nullptr; // invalid timeoutID
       }
     }
+    static inline id_ptr_pair getUniqueIdAndPtr(AsyncHandle &&handle) {
+      auto timeoutID = getUniqueId(std::move(handle));
+      auto ptr = fromId(timeoutID);
+      return std::make_pair(timeoutID, ptr);
+    }
 
     /**
      * @brief Get the underlying `asyncio.Handle` Python object
@@ -71,8 +82,44 @@ public:
       Py_INCREF(_handle); // otherwise the object would be GC-ed as the AsyncHandle destructor decreases the reference count
       return _handle;
     }
+
+    /**
+     * @brief Replace the underlying `asyncio.Handle` Python object with the provided value
+     * @return the old `asyncio.Handle` object
+     */
+    inline PyObject *swap(PyObject *newHandleObject) {
+      return std::exchange(_handle, newHandleObject);
+    }
+
+    /**
+     * @brief Getter for if the timer has been ref'ed
+     */
+    inline bool hasRef() {
+      return _refed;
+    }
+
+    /**
+     * @brief Ref the timer so that the event-loop won't exit as long as the timer is active
+     */
+    inline void addRef() {
+      if (!_refed) {
+        _refed = true;
+        PyEventLoop::_locker->incCounter();
+      }
+    }
+
+    /**
+     * @brief Unref the timer so that the event-loop can exit
+     */
+    inline void removeRef() {
+      if (_refed) {
+        _refed = false;
+        PyEventLoop::_locker->decCounter();
+      }
+    }
   protected:
     PyObject *_handle;
+    std::atomic_bool _refed = false;
   };
 
   /**
@@ -85,9 +132,9 @@ public:
    * @brief Schedule a job to the Python event-loop, with the given delay
    * @param jobFn - The JS event-loop job converted to a Python function
    * @param delaySeconds - The job function will be called after the given number of seconds
-   * @return a AsyncHandle, the value can be safely ignored
+   * @return the timeoutId and a pointer to the AsyncHandle
    */
-  AsyncHandle enqueueWithDelay(PyObject *jobFn, double delaySeconds);
+  [[nodiscard]] AsyncHandle::id_ptr_pair enqueueWithDelay(PyObject *jobFn, double delaySeconds);
 
   /**
    * @brief C++ wrapper for Python `asyncio.Future` class

python/pythonmonkey/builtin_modules/internal-binding.d.ts

@@ -45,6 +45,21 @@ declare function internalBinding(namespace: "timers"): {
    * internal binding helper for the `clearTimeout` global function
    */
   cancelByTimeoutId(timeoutId: number): void;
+
+  /**
+   * internal binding helper for if a timer object has been ref'ed
+   */
+  timerHasRef(timeoutId: number): boolean;
+
+  /**
+   * internal binding helper for ref'ing the timer
+   */
+  timerAddRef(timeoutId: number): void;
+
+  /**
+   * internal binding helper for unref'ing the timer
+   */
+  timerRemoveRef(timeoutId: number): void;
 };
 
 export = internalBinding;