From 1d0543566182d3d1537db965d67b1e415db4705f Mon Sep 17 00:00:00 2001 From: ijl Date: Thu, 7 Sep 2023 21:22:20 +0000 Subject: [PATCH] assert, test, and CI misc --- .github/workflows/debug.yaml | 5 ++- .github/workflows/linux.yaml | 14 +++++-- Cargo.lock | 66 ++++++++++++++++++++++----------- README.md | 2 +- ci/azure-macos.yml | 2 + ci/azure-pipelines.yml | 2 +- ci/azure-win.yml | 2 + integration/client | 3 +- integration/requirements.txt | 2 +- integration/run | 2 + script/develop | 2 +- script/pytest | 2 +- src/deserialize/cache.rs | 2 +- src/deserialize/deserializer.rs | 1 + src/deserialize/json.rs | 6 ++- src/deserialize/pyobject.rs | 2 + src/deserialize/utf8.rs | 1 + src/deserialize/yyjson.rs | 4 +- src/ffi/immortal.rs | 27 ++++++++++++++ src/ffi/mod.rs | 4 ++ src/lib.rs | 4 ++ src/serialize/dataclass.rs | 23 ++++++++++-- src/serialize/default.rs | 2 +- src/serialize/dict.rs | 4 +- src/serialize/pyenum.rs | 8 ++-- src/serialize/writer.rs | 6 +-- src/typeref.rs | 3 +- src/util.rs | 10 +++-- test/test_datetime.py | 14 +++---- test/test_default.py | 40 +++++++++++++++++++- test/test_memory.py | 46 ++++++----------------- 31 files changed, 214 insertions(+), 97 deletions(-) create mode 100644 src/ffi/immortal.rs diff --git a/.github/workflows/debug.yaml b/.github/workflows/debug.yaml index 438ebc04..9cec5e7e 100644 --- a/.github/workflows/debug.yaml +++ b/.github/workflows/debug.yaml @@ -9,7 +9,7 @@ jobs: matrix: rust: [ { version: "1.60" }, # MSRV - { version: "nightly-2023-06-30" }, + { version: "nightly-2023-08-30" }, ] python: [ { version: '3.11', abi: 'cp311-cp311' }, @@ -43,6 +43,9 @@ jobs: - run: python -m pip install --user -r test/requirements.txt -r integration/requirements.txt - run: pytest -s -rxX -v test + env: + PYTHONMALLOC: "debug" + - run: ./integration/run thread - run: ./integration/run http - run: ./integration/run init diff --git a/.github/workflows/linux.yaml b/.github/workflows/linux.yaml index dda7e214..743008d9 100644 --- a/.github/workflows/linux.yaml +++ b/.github/workflows/linux.yaml @@ -31,6 +31,9 @@ jobs: - run: python3 -m pip install --user -r test/requirements.txt -r integration/requirements.txt mypy - run: pytest -s -rxX -v test + env: + PYTHONMALLOC: "debug" + - run: ./integration/run thread - run: ./integration/run http - run: ./integration/run init @@ -69,8 +72,8 @@ jobs: options: --user 0 steps: - run: yum install -y clang lld - - run: curl https://sh.rustup.rs -sSf | sh -s -- --default-toolchain nightly-2023-06-30 --profile minimal -y - - run: rustup component add rust-src --toolchain nightly-2023-06-30-x86_64-unknown-linux-gnu + - run: curl https://sh.rustup.rs -sSf | sh -s -- --default-toolchain nightly-2023-08-30 --profile minimal -y + - run: rustup component add rust-src --toolchain nightly-2023-08-30-x86_64-unknown-linux-gnu - uses: actions/checkout@v3 - name: build-std @@ -91,6 +94,9 @@ jobs: - run: python3 -m pip install --user -r test/requirements.txt -r integration/requirements.txt - run: pytest -s -rxX -v test + env: + PYTHONMALLOC: "debug" + - run: ./integration/run thread - run: ./integration/run http - run: ./integration/run init @@ -141,7 +147,7 @@ jobs: RUSTFLAGS: "-C target-feature=-crt-static" CARGO_UNSTABLE_SPARSE_REGISTRY: "true" with: - rust-toolchain: nightly-2023-06-30 + rust-toolchain: nightly-2023-08-30 rustup-components: rust-src target: ${{ matrix.platform.target }} manylinux: musllinux_1_1 @@ -230,7 +236,7 @@ jobs: CARGO_UNSTABLE_SPARSE_REGISTRY: "true" with: target: ${{ matrix.target.target }} - rust-toolchain: nightly-2023-06-30 + rust-toolchain: nightly-2023-08-30 rustup-components: rust-src manylinux: auto args: --release --strip --out=dist --features=no-panic,yyjson -i python${{ matrix.python.version }} diff --git a/Cargo.lock b/Cargo.lock index 8cdeaed6..aeadc68d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13,12 +13,6 @@ dependencies = [ "version_check", ] -[[package]] -name = "android-tzdata" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e999941b234f3131b00bc13c22d06e8c5ff726d1b6318ac7eb276997bbb4fef0" - [[package]] name = "arrayvec" version = "0.7.4" @@ -69,9 +63,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.0.82" +version = "1.0.83" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "305fe645edc1442a0fa8b6726ba61d422798d37a52e12eaecf4b022ebbb88f01" +checksum = "f1174fb0b6ec23863f8b971027804a42614e347eafb0a95bf0b12cdae21fc4d0" dependencies = [ "libc", ] @@ -84,11 +78,10 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" [[package]] name = "chrono" -version = "0.4.26" +version = "0.4.30" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec837a71355b28f6556dbd569b37b3f363091c0bd4b2e735674521b4c5fd9bc5" +checksum = "defd4e7873dbddba6c7c91e199c7fcb946abc4a6a4ac3195400bcfb01b5de877" dependencies = [ - "android-tzdata", "num-traits", ] @@ -108,12 +101,12 @@ dependencies = [ [[package]] name = "encoding_rs" -version = "0.8.32" +version = "0.8.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "071a31f4ee85403370b58aca746f01041ede6f0da2730960ad001edc2b71b394" +checksum = "7268b386296a025e474d5140678f75d6de9493ae55a5d709eeb9dd08149945e1" dependencies = [ "cfg-if", - "packed_simd_2", + "packed_simd", ] [[package]] @@ -143,6 +136,12 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7fc7aa29613bd6a620df431842069224d8bc9011086b1db4c0e0cd47fa03ec9a" +[[package]] +name = "libm" +version = "0.2.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f7012b1bbb0719e1097c47611d3898568c546d597c2e74d66f6087edd5233ff4" + [[package]] name = "no-panic" version = "0.1.26" @@ -161,6 +160,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f30b0abd723be7e2ffca1272140fac1a2f084c77ec3e123c192b66af1ee9e6c2" dependencies = [ "autocfg", + "libm 0.2.7", ] [[package]] @@ -195,6 +195,16 @@ dependencies = [ "version_check", ] +[[package]] +name = "packed_simd" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f9f08af0c877571712e2e3e686ad79efad9657dbf0f7c3c8ba943ff6c38932d" +dependencies = [ + "cfg-if", + "num-traits", +] + [[package]] name = "packed_simd_2" version = "0.3.8" @@ -202,7 +212,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a1914cd452d8fccd6f9db48147b29fd4ae05bea9dc5d9ad578509f72415de282" dependencies = [ "cfg-if", - "libm", + "libm 0.1.4", ] [[package]] @@ -236,9 +246,9 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.32" +version = "1.0.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50f3b39ccfb720540debaa0164757101c08ecb8d326b15358ce76a62c7e85965" +checksum = "5267fca4496028628a95160fc423a33e8b2e6af8a5302579e322e4b520293cae" dependencies = [ "proc-macro2", ] @@ -260,9 +270,23 @@ dependencies = [ [[package]] name = "serde" -version = "1.0.183" +version = "1.0.188" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf9e0fcba69a370eed61bcf2b728575f726b50b55cba78064753d708ddc7549e" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.188" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32ac8da02677876d532745a130fc9d8e6edfa81a269b107c5b00829b91d8eb3c" +checksum = "4eca7ac642d82aa35b60049a6eccb4be6be75e599bd2e9adb5f875a737654af2" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] [[package]] name = "serde_json" @@ -295,9 +319,9 @@ checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" [[package]] name = "syn" -version = "2.0.28" +version = "2.0.31" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "04361975b3f5e348b2189d8dc55bc942f278b2d482a6a0365de5bdd62d351567" +checksum = "718fa2415bcb8d8bd775917a1bf12a7931b6dfa890753378538118181e0cb398" dependencies = [ "proc-macro2", "quote", diff --git a/README.md b/README.md index 07899585..22b25ed5 100644 --- a/README.md +++ b/README.md @@ -1195,7 +1195,7 @@ It benefits from also having a C build environment to compile a faster deserialization backend. See this project's `manylinux_2_28` builds for an example using clang and LTO. -The project's own CI tests against `nightly-2023-06-30` and stable 1.60. It +The project's own CI tests against `nightly-2023-08-30` and stable 1.60. It is prudent to pin the nightly version because that channel can introduce breaking changes. diff --git a/ci/azure-macos.yml b/ci/azure-macos.yml index 8278d71e..7ada3141 100644 --- a/ci/azure-macos.yml +++ b/ci/azure-macos.yml @@ -37,6 +37,8 @@ steps: displayName: install universal2 - bash: pytest -s -rxX -v test + env: + PYTHONMALLOC: "debug" displayName: pytest - bash: ./integration/run thread displayName: thread diff --git a/ci/azure-pipelines.yml b/ci/azure-pipelines.yml index 96010a00..6f0f8f0f 100644 --- a/ci/azure-pipelines.yml +++ b/ci/azure-pipelines.yml @@ -1,5 +1,5 @@ variables: - toolchain: nightly-2023-06-30 + toolchain: nightly-2023-08-30 jobs: diff --git a/ci/azure-win.yml b/ci/azure-win.yml index 5f38ece4..6e65c241 100644 --- a/ci/azure-win.yml +++ b/ci/azure-win.yml @@ -24,6 +24,8 @@ steps: - script: python.exe -m pip install orjson --no-index --find-links=D:\a\1\s\target\wheels displayName: install - script: python.exe -m pytest -s -rxX -v test + env: + PYTHONMALLOC: "debug" displayName: pytest - script: python.exe integration\thread displayName: thread diff --git a/integration/client b/integration/client index a86b9654..f8684769 100755 --- a/integration/client +++ b/integration/client @@ -9,7 +9,7 @@ import httpx port = sys.argv[1] url = f"http://127.0.0.1:{port}" -timeout = httpx.Timeout(5.0, connect_timeout=5.0) +timeout = httpx.Timeout(5.0) client = httpx.AsyncClient(timeout=timeout) stop_time = time.time() + 5 @@ -30,3 +30,4 @@ async def main(): loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) asyncio.run(main()) +loop.close() diff --git a/integration/requirements.txt b/integration/requirements.txt index 7479be65..e5b290de 100644 --- a/integration/requirements.txt +++ b/integration/requirements.txt @@ -1,3 +1,3 @@ flask;sys_platform!="win" gunicorn;sys_platform!="win" -httpx==0.14.3;sys_platform!="win" +httpx==0.24.1;sys_platform!="win" diff --git a/integration/run b/integration/run index 70047e73..477f7076 100755 --- a/integration/run +++ b/integration/run @@ -6,6 +6,8 @@ _dir="$(dirname "${BASH_SOURCE[0]}")" to_run="${@:-thread http init}" +export PYTHONMALLOC="debug" + if [[ $to_run == *"thread"* ]]; then "${_dir}"/thread fi diff --git a/script/develop b/script/develop index 9bfd9310..6bad5db7 100755 --- a/script/develop +++ b/script/develop @@ -7,6 +7,6 @@ export CFLAGS="-O2 -fno-plt -flto=thin" export LDFLAGS="${CFLAGS} -fuse-ld=lld -Wl,--as-needed" export RUSTFLAGS="-C linker=clang -C link-arg=-fuse-ld=lld" -maturin build --release "$@" +maturin build "$@" pip install --force target/wheels/*.whl diff --git a/script/pytest b/script/pytest index 45b230d3..4d25dbe4 100755 --- a/script/pytest +++ b/script/pytest @@ -1,3 +1,3 @@ #!/bin/sh -e -pytest -s -rxX --random-order test +PYTHONMALLOC="debug" pytest -s -rxX --random-order test diff --git a/src/deserialize/cache.rs b/src/deserialize/cache.rs index ad8bb3dc..62142f77 100644 --- a/src/deserialize/cache.rs +++ b/src/deserialize/cache.rs @@ -22,9 +22,9 @@ impl CachedKey { ptr: ptr as *mut c_void, } } - pub fn get(&mut self) -> *mut pyo3_ffi::PyObject { let ptr = self.ptr as *mut pyo3_ffi::PyObject; + debug_assert!(ffi!(Py_REFCNT(ptr)) >= 1); ffi!(Py_INCREF(ptr)); ptr } diff --git a/src/deserialize/deserializer.rs b/src/deserialize/deserializer.rs index e059d00b..0675cce0 100644 --- a/src/deserialize/deserializer.rs +++ b/src/deserialize/deserializer.rs @@ -8,6 +8,7 @@ use std::ptr::NonNull; pub fn deserialize( ptr: *mut pyo3_ffi::PyObject, ) -> Result, DeserializeError<'static>> { + debug_assert!(ffi!(Py_REFCNT(ptr)) >= 1); let buffer = read_input_to_buf(ptr)?; if unlikely!(buffer.len() == 2) { diff --git a/src/deserialize/json.rs b/src/deserialize/json.rs index 4863accd..4381f5be 100644 --- a/src/deserialize/json.rs +++ b/src/deserialize/json.rs @@ -136,8 +136,10 @@ impl<'de> Visitor<'de> for JsonValue { str_hash!(pykey), ) }; - py_decref_without_destroy!(pykey); - py_decref_without_destroy!(pyval.as_ptr()); + debug_assert!(ffi!(Py_REFCNT(pykey)) >= 2); + reverse_pydict_incref!(pykey); + debug_assert!(ffi!(Py_REFCNT(pyval.as_ptr())) >= 2); + reverse_pydict_incref!(pyval.as_ptr()); } Ok(nonnull!(dict_ptr)) } diff --git a/src/deserialize/pyobject.rs b/src/deserialize/pyobject.rs index a63d6e7b..49766b14 100644 --- a/src/deserialize/pyobject.rs +++ b/src/deserialize/pyobject.rs @@ -29,6 +29,8 @@ pub fn get_unicode_key(key_str: &str) -> *mut pyo3_ffi::PyObject { ); pykey = entry.get(); } + debug_assert!(ffi!(Py_REFCNT(pykey)) >= 1); + debug_assert!(unsafe { (*pykey.cast::()).hash != -1 }); pykey } diff --git a/src/deserialize/utf8.rs b/src/deserialize/utf8.rs index 1791e525..ca1451af 100644 --- a/src/deserialize/utf8.rs +++ b/src/deserialize/utf8.rs @@ -56,6 +56,7 @@ pub fn read_input_to_buf( buffer = unsafe { std::slice::from_raw_parts(as_str.as_ptr(), as_str.len()) }; } else if unlikely!(is_type!(obj_type_ptr, MEMORYVIEW_TYPE)) { let membuf = unsafe { PyMemoryView_GET_BUFFER(ptr) }; + debug_assert!(ffi!(Py_REFCNT(membuf as *mut pyo3_ffi::PyObject)) >= 1); if unsafe { pyo3_ffi::PyBuffer_IsContiguous(membuf, b'C' as c_char) == 0 } { return Err(DeserializeError::invalid(Cow::Borrowed( "Input type memoryview must be a C contiguous buffer", diff --git a/src/deserialize/yyjson.rs b/src/deserialize/yyjson.rs index 811a8e60..862da0b6 100644 --- a/src/deserialize/yyjson.rs +++ b/src/deserialize/yyjson.rs @@ -185,8 +185,8 @@ fn parse_yy_object(elem: *mut yyjson_val) -> NonNull { let _ = unsafe { pyo3_ffi::_PyDict_SetItem_KnownHash(dict, pykey, pyval, str_hash!(pykey)) }; - py_decref_without_destroy!(pykey); - py_decref_without_destroy!(pyval); + reverse_pydict_incref!(pykey); + reverse_pydict_incref!(pyval); } nonnull!(dict) } diff --git a/src/ffi/immortal.rs b/src/ffi/immortal.rs new file mode 100644 index 00000000..a97fbc8e --- /dev/null +++ b/src/ffi/immortal.rs @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: (Apache-2.0 OR MIT) +// copied from PyO3 when 0.19.2 was latest + +use pyo3_ffi::{PyObject, Py_ssize_t}; +use std::os::raw::{c_int, c_uint}; + +#[cfg(Py_3_12)] +pub const _Py_IMMORTAL_REFCNT: Py_ssize_t = { + if cfg!(target_pointer_width = "64") { + c_uint::MAX as Py_ssize_t + } else { + // for 32-bit systems, use the lower 30 bits (see comment in CPython's object.h) + (c_uint::MAX >> 2) as Py_ssize_t + } +}; + +#[inline(always)] +#[cfg(all(Py_3_12, target_pointer_width = "64"))] +pub unsafe fn _Py_IsImmortal(op: *mut PyObject) -> c_int { + (((*op).ob_refcnt.ob_refcnt as crate::PY_INT32_T) < 0) as c_int +} + +#[inline(always)] +#[cfg(all(Py_3_12, target_pointer_width = "32"))] +pub unsafe fn _Py_IsImmortal(op: *mut PyObject) -> c_int { + ((*op).ob_refcnt.ob_refcnt == _Py_IMMORTAL_REFCNT) as c_int +} diff --git a/src/ffi/mod.rs b/src/ffi/mod.rs index b59d102c..c82cf032 100644 --- a/src/ffi/mod.rs +++ b/src/ffi/mod.rs @@ -4,6 +4,8 @@ mod buffer; mod bytes; mod dict; mod fragment; +#[cfg(Py_3_12)] +mod immortal; mod list; mod long; @@ -11,5 +13,7 @@ pub use buffer::*; pub use bytes::*; pub use dict::*; pub use fragment::{orjson_fragmenttype_new, Fragment}; +#[cfg(Py_3_12)] +pub use immortal::_Py_IsImmortal; pub use list::PyListIter; pub use long::{pylong_is_unsigned, pylong_is_zero}; diff --git a/src/lib.rs b/src/lib.rs index 975914bc..7728d452 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -218,6 +218,7 @@ fn raise_loads_exception(err: deserialize::DeserializeError) -> *mut PyObject { PyTuple_SET_ITEM(args, 1, doc); PyTuple_SET_ITEM(args, 2, pos); PyErr_SetObject(typeref::JsonDecodeError, args); + debug_assert!(ffi!(Py_REFCNT(args)) == 2); Py_DECREF(args); }; null_mut() @@ -231,6 +232,7 @@ fn raise_dumps_exception_fixed(msg: &str) -> *mut PyObject { let err_msg = PyUnicode_FromStringAndSize(msg.as_ptr() as *const c_char, msg.len() as isize); PyErr_SetObject(typeref::JsonEncodeError, err_msg); + debug_assert!(ffi!(Py_REFCNT(err_msg)) == 2); Py_DECREF(err_msg); }; null_mut() @@ -247,6 +249,7 @@ fn raise_dumps_exception_dynamic(err: &String) -> *mut PyObject { let err_msg = PyUnicode_FromStringAndSize(err.as_ptr() as *const c_char, err.len() as isize); PyErr_SetObject(typeref::JsonEncodeError, err_msg); + debug_assert!(ffi!(Py_REFCNT(err_msg)) == 2); Py_DECREF(err_msg); if !cause_exc.is_null() { @@ -272,6 +275,7 @@ fn raise_dumps_exception_dynamic(err: &String) -> *mut PyObject { let err_msg = PyUnicode_FromStringAndSize(err.as_ptr() as *const c_char, err.len() as isize); PyErr_SetObject(typeref::JsonEncodeError, err_msg); + debug_assert!(ffi!(Py_REFCNT(err_msg)) == 2); Py_DECREF(err_msg); let mut tp: *mut PyObject = null_mut(); let mut val: *mut PyObject = null_mut(); diff --git a/src/serialize/dataclass.rs b/src/serialize/dataclass.rs index 13d59533..aa805c4e 100644 --- a/src/serialize/dataclass.rs +++ b/src/serialize/dataclass.rs @@ -48,7 +48,7 @@ impl Serialize for DataclassGenericSerializer { } let dict = ffi!(PyObject_GetAttr(self.ptr, DICT_STR)); let ob_type = ob_type!(self.ptr); - if unlikely!(dict.is_null() || pydict_contains!(ob_type, SLOTS_STR)) { + if unlikely!(dict.is_null()) { ffi!(PyErr_Clear()); DataclassFallbackSerializer::new( self.ptr, @@ -58,16 +58,28 @@ impl Serialize for DataclassGenericSerializer { self.default, ) .serialize(serializer) - } else { + } else if pydict_contains!(ob_type, SLOTS_STR) { + let ret = DataclassFallbackSerializer::new( + self.ptr, + self.opts, + self.default_calls, + self.recursion, + self.default, + ) + .serialize(serializer); ffi!(Py_DECREF(dict)); - DataclassFastSerializer::new( + ret + } else { + let ret = DataclassFastSerializer::new( dict, self.opts, self.default_calls, self.recursion, self.default, ) - .serialize(serializer) + .serialize(serializer); + ffi!(Py_DECREF(dict)); + ret } } } @@ -166,6 +178,7 @@ impl Serialize for DataclassFallbackSerializer { S: Serializer, { let fields = ffi!(PyObject_GetAttr(self.ptr, DATACLASS_FIELDS_STR)); + debug_assert!(ffi!(Py_REFCNT(fields)) >= 2); ffi!(Py_DECREF(fields)); let len = ffi!(Py_SIZE(fields)) as usize; if unlikely!(len == 0) { @@ -174,6 +187,7 @@ impl Serialize for DataclassFallbackSerializer { let mut map = serializer.serialize_map(None).unwrap(); for (attr, field) in PyDictIter::from_pyobject(fields) { let field_type = ffi!(PyObject_GetAttr(field.as_ptr(), FIELD_TYPE_STR)); + debug_assert!(ffi!(Py_REFCNT(field_type)) >= 2); ffi!(Py_DECREF(field_type)); if unsafe { field_type as *mut pyo3_ffi::PyTypeObject != FIELD_TYPE } { continue; @@ -188,6 +202,7 @@ impl Serialize for DataclassFallbackSerializer { } let value = ffi!(PyObject_GetAttr(self.ptr, attr.as_ptr())); + debug_assert!(ffi!(Py_REFCNT(value)) >= 2); ffi!(Py_DECREF(value)); let pyvalue = PyObjectSerializer::new( value, diff --git a/src/serialize/default.rs b/src/serialize/default.rs index 0f093a7b..6f2d0982 100644 --- a/src/serialize/default.rs +++ b/src/serialize/default.rs @@ -58,7 +58,7 @@ impl Serialize for DefaultSerializer { callable.as_ptr(), std::ptr::addr_of!(self.ptr), pyo3_ffi::PyVectorcall_NARGS(1) as usize, - std::ptr::null_mut() as *mut pyo3_ffi::PyObject, + std::ptr::null_mut(), ) }; if unlikely!(default_obj.is_null()) { diff --git a/src/serialize/dict.rs b/src/serialize/dict.rs index f2a8d54e..a2517e06 100644 --- a/src/serialize/dict.rs +++ b/src/serialize/dict.rs @@ -293,8 +293,10 @@ impl DictNonStrKey { } ObType::Enum => { let value = ffi!(PyObject_GetAttr(key, VALUE_STR)); + debug_assert!(ffi!(Py_REFCNT(value)) >= 2); + let ret = Self::pyobject_to_string(value, opts); ffi!(Py_DECREF(value)); - Self::pyobject_to_string(value, opts) + ret } ObType::Str => { // because of ObType::Enum diff --git a/src/serialize/pyenum.rs b/src/serialize/pyenum.rs index 0da1a46b..1972f8a2 100644 --- a/src/serialize/pyenum.rs +++ b/src/serialize/pyenum.rs @@ -39,14 +39,16 @@ impl Serialize for EnumSerializer { S: Serializer, { let value = ffi!(PyObject_GetAttr(self.ptr, VALUE_STR)); - ffi!(Py_DECREF(value)); - PyObjectSerializer::new( + debug_assert!(ffi!(Py_REFCNT(value)) >= 2); + let ret = PyObjectSerializer::new( value, self.opts, self.default_calls, self.recursion, self.default, ) - .serialize(serializer) + .serialize(serializer); + ffi!(Py_DECREF(value)); + ret } } diff --git a/src/serialize/writer.rs b/src/serialize/writer.rs index b033784b..ab953805 100644 --- a/src/serialize/writer.rs +++ b/src/serialize/writer.rs @@ -87,7 +87,7 @@ impl std::io::Write for BytesWriter { self.grow(end_length); } unsafe { - std::ptr::copy_nonoverlapping(buf.as_ptr() as *const u8, self.buffer_ptr(), to_write); + std::ptr::copy_nonoverlapping(buf.as_ptr(), self.buffer_ptr(), to_write); }; self.len = end_length; Ok(()) @@ -177,7 +177,7 @@ impl WriteExt for &mut BytesWriter { unsafe { let ptr = self.buffer_ptr(); std::ptr::write(ptr, b'"'); - std::ptr::copy_nonoverlapping(val.as_ptr() as *const u8, ptr.add(1), to_write); + std::ptr::copy_nonoverlapping(val.as_ptr(), ptr.add(1), to_write); std::ptr::write(ptr.add(to_write + 1), b'"'); }; self.len = end_length; @@ -190,7 +190,7 @@ impl WriteExt for &mut BytesWriter { ) -> std::result::Result<(), std::io::Error> { let to_write = val.len(); unsafe { - std::ptr::copy_nonoverlapping(val.as_ptr() as *const u8, self.buffer_ptr(), to_write); + std::ptr::copy_nonoverlapping(val.as_ptr(), self.buffer_ptr(), to_write); }; self.len += to_write; Ok(()) diff --git a/src/typeref.rs b/src/typeref.rs index 7bfaf586..2207936e 100644 --- a/src/typeref.rs +++ b/src/typeref.rs @@ -208,8 +208,7 @@ fn _init_typerefs_impl() -> bool { unsafe fn look_up_json_exc() -> *mut PyObject { let module = PyImport_ImportModule("json\0".as_ptr() as *const c_char); let module_dict = PyObject_GenericGetDict(module, null_mut()); - let ptr = PyMapping_GetItemString(module_dict, "JSONDecodeError\0".as_ptr() as *const c_char) - as *mut PyObject; + let ptr = PyMapping_GetItemString(module_dict, "JSONDecodeError\0".as_ptr() as *const c_char); let res = pyo3_ffi::PyErr_NewException( "orjson.JSONDecodeError\0".as_ptr() as *const c_char, ptr, diff --git a/src/util.rs b/src/util.rs index 0a1c06ce..38066313 100644 --- a/src/util.rs +++ b/src/util.rs @@ -57,18 +57,22 @@ macro_rules! str_from_slice { } #[cfg(Py_3_12)] -macro_rules! py_decref_without_destroy { +macro_rules! reverse_pydict_incref { ($op:expr) => { unsafe { - (*$op).ob_refcnt.ob_refcnt -= 1; + if !_Py_IsImmortal($op) { + debug_assert!(ffi!(Py_REFCNT($op)) >= 2); + (*$op).ob_refcnt.ob_refcnt -= 1; + } } }; } #[cfg(not(Py_3_12))] -macro_rules! py_decref_without_destroy { +macro_rules! reverse_pydict_incref { ($op:expr) => { unsafe { + debug_assert!(ffi!(Py_REFCNT($op)) >= 2); (*$op).ob_refcnt -= 1; } }; diff --git a/test/test_datetime.py b/test/test_datetime.py index 7ee3b79c..b40ffd34 100644 --- a/test/test_datetime.py +++ b/test/test_datetime.py @@ -208,7 +208,7 @@ def test_datetime_zoneinfo_negative(self): == b'["2018-06-01T02:03:04-04:00"]' ) - @pytest.mark.skipif(pendulum is None, reason="pendulum install broken on win") + @pytest.mark.skipif(pendulum is None, reason="pendulum not installed") def test_datetime_pendulum_utc(self): """ datetime.datetime UTC @@ -252,7 +252,7 @@ def test_datetime_pytz_positive(self): == b'["2018-01-01T02:03:04+08:00"]' ) - @pytest.mark.skipif(pendulum is None, reason="pendulum install broken on win") + @pytest.mark.skipif(pendulum is None, reason="pendulum not installed") def test_datetime_pendulum_positive(self): """ datetime.datetime positive UTC @@ -291,7 +291,7 @@ def test_datetime_pytz_negative_dst(self): == b'["2018-06-01T02:03:04-04:00"]' ) - @pytest.mark.skipif(pendulum is None, reason="pendulum install broken on win") + @pytest.mark.skipif(pendulum is None, reason="pendulum not installed") def test_datetime_pendulum_negative_dst(self): """ datetime.datetime negative UTC DST @@ -360,7 +360,7 @@ def test_datetime_pytz_negative_non_dst(self): == b'["2018-12-01T02:03:04-05:00"]' ) - @pytest.mark.skipif(pendulum is None, reason="pendulum install broken on win") + @pytest.mark.skipif(pendulum is None, reason="pendulum not installed") def test_datetime_pendulum_negative_non_dst(self): """ datetime.datetime negative UTC non-DST @@ -429,7 +429,7 @@ def test_datetime_pytz_partial_hour(self): == b'["2018-12-01T02:03:04+10:30"]' ) - @pytest.mark.skipif(pendulum is None, reason="pendulum install broken on win") + @pytest.mark.skipif(pendulum is None, reason="pendulum not installed") def test_datetime_pendulum_partial_hour(self): """ datetime.datetime UTC offset partial hour @@ -452,7 +452,7 @@ def test_datetime_pendulum_partial_hour(self): == b'["2018-12-01T02:03:04+10:30"]' ) - @pytest.mark.skipif(pendulum is None, reason="pendulum install broken on win") + @pytest.mark.skipif(pendulum is None, reason="pendulum not installed") def test_datetime_partial_second_pendulum_supported(self): """ datetime.datetime UTC offset round seconds @@ -663,7 +663,7 @@ def test_datetime_utc_z_with_tz(self): in AMSTERDAM_1937_DATETIMES_WITH_Z ) - @pytest.mark.skipif(pendulum is None, reason="pendulum install broken on win") + @pytest.mark.skipif(pendulum is None, reason="pendulum not installed") def test_datetime_roundtrip(self): """ datetime.datetime parsed by pendulum diff --git a/test/test_default.py b/test/test_default.py index a110a662..a8f2bcc1 100644 --- a/test/test_default.py +++ b/test/test_default.py @@ -1,5 +1,6 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) +import datetime import sys import uuid @@ -264,10 +265,24 @@ def test_default_recursion_infinite(self): def default(obj): return obj + refcount = sys.getrefcount(ref) with pytest.raises(orjson.JSONEncodeError): orjson.dumps(ref, default=default) + assert sys.getrefcount(ref) == refcount + + def test_reference_cleanup_default_custom_pass(self): + ref = Custom() + + def default(obj): + if isinstance(ref, Custom): + return str(ref) + raise TypeError + + refcount = sys.getrefcount(ref) + orjson.dumps(ref, default=default) + assert sys.getrefcount(ref) == refcount - def test_reference_cleanup_default(self): + def test_reference_cleanup_default_custom_error(self): """ references to encoded objects are cleaned up """ @@ -276,10 +291,31 @@ def test_reference_cleanup_default(self): def default(obj): raise TypeError + refcount = sys.getrefcount(ref) with pytest.raises(orjson.JSONEncodeError): orjson.dumps(ref, default=default) + assert sys.getrefcount(ref) == refcount - assert sys.getrefcount(ref) == 2 # one for ref, one for default + def test_reference_cleanup_default_subclass(self): + ref = datetime.datetime(1970, 1, 1, 0, 0, 0) + + def default(obj): + if isinstance(ref, datetime.datetime): + return repr(ref) + raise TypeError + + refcount = sys.getrefcount(ref) + orjson.dumps(ref, option=orjson.OPT_PASSTHROUGH_DATETIME, default=default) + assert sys.getrefcount(ref) == refcount + + def test_reference_cleanup_default_subclass_lambda(self): + ref = uuid.uuid4() + + refcount = sys.getrefcount(ref) + orjson.dumps( + ref, option=orjson.OPT_PASSTHROUGH_DATETIME, default=lambda val: str(val) + ) + assert sys.getrefcount(ref) == refcount @pytest.mark.skipif(numpy is None, reason="numpy is not installed") def test_default_numpy(self): diff --git a/test/test_memory.py b/test/test_memory.py index 141fe55d..9aed19c2 100644 --- a/test/test_memory.py +++ b/test/test_memory.py @@ -69,9 +69,7 @@ class Unsupported: class TestMemory: - @pytest.mark.skipif( - psutil is None, reason="psutil install broken on win, python3.9, Azure" - ) + @pytest.mark.skipif(psutil is None, reason="psutil not installed") def test_memory_loads(self): """ loads() memory leak @@ -87,9 +85,7 @@ def test_memory_loads(self): gc.collect() assert proc.memory_info().rss <= mem + MAX_INCREASE - @pytest.mark.skipif( - psutil is None, reason="psutil install broken on win, python3.9, Azure" - ) + @pytest.mark.skipif(psutil is None, reason="psutil not installed") def test_memory_loads_memoryview(self): """ loads() memory leak using memoryview @@ -106,9 +102,7 @@ def test_memory_loads_memoryview(self): gc.collect() assert proc.memory_info().rss <= mem + MAX_INCREASE - @pytest.mark.skipif( - psutil is None, reason="psutil install broken on win, python3.9, Azure" - ) + @pytest.mark.skipif(psutil is None, reason="psutil not installed") def test_memory_dumps(self): """ dumps() memory leak @@ -126,9 +120,7 @@ def test_memory_dumps(self): assert proc.memory_info().rss <= mem + MAX_INCREASE assert proc.memory_info().rss <= mem + MAX_INCREASE - @pytest.mark.skipif( - psutil is None, reason="psutil install broken on win, python3.9, Azure" - ) + @pytest.mark.skipif(psutil is None, reason="psutil not installed") def test_memory_loads_exc(self): """ loads() memory leak exception without a GC pause @@ -147,9 +139,7 @@ def test_memory_loads_exc(self): assert proc.memory_info().rss <= mem + MAX_INCREASE gc.enable() - @pytest.mark.skipif( - psutil is None, reason="psutil install broken on win, python3.9, Azure" - ) + @pytest.mark.skipif(psutil is None, reason="psutil not installed") def test_memory_dumps_exc(self): """ dumps() memory leak exception without a GC pause @@ -169,9 +159,7 @@ def test_memory_dumps_exc(self): assert proc.memory_info().rss <= mem + MAX_INCREASE gc.enable() - @pytest.mark.skipif( - psutil is None, reason="psutil install broken on win, python3.9, Azure" - ) + @pytest.mark.skipif(psutil is None, reason="psutil not installed") def test_memory_dumps_default(self): """ dumps() default memory leak @@ -196,9 +184,7 @@ def __str__(self): gc.collect() assert proc.memory_info().rss <= mem + MAX_INCREASE - @pytest.mark.skipif( - psutil is None, reason="psutil install broken on win, python3.9, Azure" - ) + @pytest.mark.skipif(psutil is None, reason="psutil not installed") def test_memory_dumps_dataclass(self): """ dumps() dataclass memory leak @@ -217,7 +203,7 @@ def test_memory_dumps_dataclass(self): @pytest.mark.skipif( psutil is None or pytz is None, - reason="psutil install broken on win, python3.9, Azure", + reason="psutil not installed", ) def test_memory_dumps_pytz_tzinfo(self): """ @@ -236,9 +222,7 @@ def test_memory_dumps_pytz_tzinfo(self): gc.collect() assert proc.memory_info().rss <= mem + MAX_INCREASE - @pytest.mark.skipif( - psutil is None, reason="psutil install broken on win, python3.9, Azure" - ) + @pytest.mark.skipif(psutil is None, reason="psutil not installed") def test_memory_loads_keys(self): """ loads() memory leak with number of keys causing cache eviction @@ -257,9 +241,7 @@ def test_memory_loads_keys(self): gc.collect() assert proc.memory_info().rss <= mem + MAX_INCREASE - @pytest.mark.skipif( - psutil is None, reason="psutil install broken on win, python3.9, Azure" - ) + @pytest.mark.skipif(psutil is None, reason="psutil not installed") @pytest.mark.skipif(numpy is None, reason="numpy is not installed") def test_memory_dumps_numpy(self): """ @@ -278,9 +260,7 @@ def test_memory_dumps_numpy(self): gc.collect() assert proc.memory_info().rss <= mem + MAX_INCREASE - @pytest.mark.skipif( - psutil is None, reason="psutil install broken on win, python3.9, Azure" - ) + @pytest.mark.skipif(psutil is None, reason="psutil not installed") @pytest.mark.skipif(pandas is None, reason="pandas is not installed") def test_memory_dumps_pandas(self): """ @@ -299,9 +279,7 @@ def test_memory_dumps_pandas(self): gc.collect() assert proc.memory_info().rss <= mem + MAX_INCREASE - @pytest.mark.skipif( - psutil is None, reason="psutil install broken on win, python3.9, Azure" - ) + @pytest.mark.skipif(psutil is None, reason="psutil not installed") def test_memory_dumps_fragment(self): """ dumps() Fragment memory leak