Commit 5cfb09d
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -157,6 +157,69 @@ mutated from multiple threads without locks; no | |||
| 157 | 157 | but the test matrix exercises 3.14t, so any new Cython module | |
| 158 | 158 | needs to keep working there. | |
| 159 | 159 | ||
| 160 | + ## Cython gotchas | ||
| 161 | + | ||
| 162 | + Non-obvious traps in the `.py` + `.pxd` setup that work fine in | ||
| 163 | + pure-Python mode but break or silently misbehave in the shipped | ||
| 164 | + Cython wheels. Distilled from the patterns that already exist in | ||
| 165 | + this repo's `.pxd` files and from incidents in sibling Cython- | ||
| 166 | + accelerated projects. | ||
| 167 | + | ||
| 168 | + - **`cdef`-typed module constants are not Python-importable.** | ||
| 169 | + Declaring `cdef unsigned int _ANSWER_STRATEGY_POINTER` in | ||
| 170 | + `query_handler.pxd` makes Cython treat | ||
| 171 | + `_ANSWER_STRATEGY_POINTER = 1` in `query_handler.py` as a C int | ||
| 172 | + assignment; the Python module dict never gets the binding. | ||
| 173 | + `from zeroconf._handlers.query_handler import | ||
| 174 | + _ANSWER_STRATEGY_POINTER` succeeds in pure-Python but raises | ||
| 175 | + `ImportError` under Cython. If you need the value visible from | ||
| 176 | + Python (e.g. a test wants to assert on it), define both names — | ||
| 177 | + a public `ANSWER_STRATEGY_POINTER = 1` Python binding plus a | ||
| 178 | + `cdef`-typed `_ANSWER_STRATEGY_POINTER = ANSWER_STRATEGY_POINTER` | ||
| 179 | + alias for hot-path comparisons. | ||
| 180 | + | ||
| 181 | + - **Match the existing `unsigned int` convention for length, TTL, | ||
| 182 | + type/class, and offset fields.** `_protocol/incoming.pxd`, | ||
| 183 | + `_cache.pxd`, and `_handlers/*.pxd` already declare these as | ||
| 184 | + `unsigned int` end-to-end. Introducing a `cdef int` return that | ||
| 185 | + carries a value originally decoded into `unsigned int` flips | ||
| 186 | + sign for any value with bit 31 set — TTL is a 32-bit DNS field | ||
| 187 | + (RFC 1035 §3.2.1, interpreted as unsigned), so a large TTL | ||
| 188 | + passed back through a `cdef int` boundary becomes negative and | ||
| 189 | + trips `< 0` sentinel branches. Stay with `unsigned int` across | ||
| 190 | + the whole call chain; if you need a real sentinel, return an | ||
| 191 | + explicit value (`UINT_MAX`, a dedicated constant) and check for | ||
| 192 | + it by equality. | ||
| 193 | + | ||
| 194 | + - **Module-level Python int constants force `PyLong_AsLong` on | ||
| 195 | + every hot-path comparison.** `if record.type == _TYPE_PTR` | ||
| 196 | + compiles to a Python attribute lookup + `PyLong_AsLong` per | ||
| 197 | + call when `_TYPE_PTR` is just a `.py`-level binding. The repo | ||
| 198 | + already follows the right pattern — `_cache.pxd` / | ||
| 199 | + `record_manager.pxd` declare `cdef unsigned int _TYPE_PTR`, | ||
| 200 | + `_DNS_PTR_MIN_TTL`, `_MIN_SCHEDULED_RECORD_EXPIRATION`, etc. | ||
| 201 | + When adding a new size / TTL / type constant from `const.py` | ||
| 202 | + to a `cdef` hot path in `_protocol/`, `_cache`, `_handlers/`, | ||
| 203 | + or `_listener`, add the `cdef`-typed alias to the corresponding | ||
| 204 | + `.pxd` at the same time. | ||
| 205 | + | ||
| 206 | + - **Sign-compare warnings in generated C are real.** `gcc`/ | ||
| 207 | + `clang` warns when comparing `unsigned int` with `int` because | ||
| 208 | + the signed value is implicitly converted to unsigned for the | ||
| 209 | + compare — a negative value becomes a huge positive. Match the | ||
| 210 | + signedness of compared operands in the `.pxd` (e.g. if the | ||
| 211 | + local is `unsigned int`, declare the constant as | ||
| 212 | + `cdef unsigned int`; if the local is `int`, declare it | ||
| 213 | + `cdef int`). The warning predicts the unsigned -> signed | ||
| 214 | + overflow class of bug. | ||
| 215 | + | ||
| 216 | + - **CodSpeed regressions only show up in the Cython build.** | ||
| 217 | + Pure-Python (`SKIP_CYTHON=1`) tests can pass while production | ||
| 218 | + wire-format hot paths regress. Trust the CodSpeed check on PRs | ||
| 219 | + that touch any file in `TO_CYTHONIZE`; rebuild in place with | ||
| 220 | + `REQUIRE_CYTHON=1 poetry install --only=main,dev` before | ||
| 221 | + pushing if perf-sensitive code changed. | ||
| 222 | + | ||
| 160 | 223 | ## Reporting security issues | |
| 161 | 224 | ||
| 162 | 225 | Suspected security vulnerabilities go through GitHub's [private | |
0 commit comments