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

pyo3: bump to version 0.14.0 #6154

Merged
merged 1 commit into from
Jul 4, 2021
Merged

Conversation

jameshilliard
Copy link
Contributor

No description provided.

@alex
Copy link
Member

alex commented Jul 3, 2021

You're not dependabot!

alex
alex previously approved these changes Jul 3, 2021
@alex
Copy link
Member

alex commented Jul 4, 2021

Uh oh, segfault! Bug in the new pyo3?

@jameshilliard
Copy link
Contributor Author

Bug in the new pyo3?

Maybe, trying to isolate what's happening.

@alex
Copy link
Member

alex commented Jul 4, 2021

Looks like the failure is pypy specific.

@jameshilliard
Copy link
Contributor Author

Looks like the failure is pypy specific.

Yeah, this is the backtrace I'm seeing:

(lldb) bt all
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=EXC_I386_GPFLT)
  * frame #0: 0x00000001063613f8 _rust.pypy37-pp73-darwin.so`pyo3::types::list::_$LT$impl$u20$pyo3..conversion..IntoPy$LT$pyo3..instance..Py$LT$pyo3..types..any..PyAny$GT$$GT$$u20$for$u20$alloc..vec..Vec$LT$T$GT$$GT$::into_py::h296574e22a733a5d + 104
    frame #1: 0x000000010634bcce _rust.pypy37-pp73-darwin.so`cryptography_rust::asn1::_$LT$impl$u20$pyo3..class..impl_..PyClassDescriptors$LT$cryptography_rust..asn1..TestCertificate$GT$$u20$for$u20$pyo3..class..impl_..PyClassImplCollector$LT$cryptography_rust..asn1..TestCertificate$GT$$GT$::py_class_descriptors::METHODS::__wrap::_$u7b$$u7b$closure$u7d$$u7d$::hf8a4c5bfd6f9f5c5 (.llvm.16029029693464088663) + 382
    frame #2: 0x0000000106349164 _rust.pypy37-pp73-darwin.so`cryptography_rust::asn1::_$LT$impl$u20$pyo3..class..impl_..PyClassDescriptors$LT$cryptography_rust..asn1..TestCertificate$GT$$u20$for$u20$pyo3..class..impl_..PyClassImplCollector$LT$cryptography_rust..asn1..TestCertificate$GT$$GT$::py_class_descriptors::METHODS::__wrap::h87cc0ef8b89c0036 (.llvm.6430969857765583387) + 116
    frame #3: 0x0000000100804748 libpypy3-c.dylib`___lldb_unnamed_symbol17573$$libpypy3-c.dylib + 144

@alex
Copy link
Member

alex commented Jul 4, 2021

I guess the next step would be to try to minimize this and report to pyo3. (cc: @davidhewitt)

@jameshilliard
Copy link
Contributor Author

I guess the next step would be to try to minimize this and report to pyo3. (cc: @davidhewitt)

This should reproduce it:

pytest tests/x509/test_x509.py::TestRSACertificateRequest::test_build_cert_printable_string_country_name

@jameshilliard
Copy link
Contributor Author

FYI this probably isn't limited to pypy, I've reproduced a crash locally with cpython 3.8 and 3.9. Working on getting traces from that.

@jameshilliard
Copy link
Contributor Author

Hmm, is this test supposed to crash a python process?

pytest tests/hazmat/backends/test_openssl_memleak.py::TestAssertNoMemoryLeaks::test_errors

@alex
Copy link
Member

alex commented Jul 4, 2021 via email

@jameshilliard
Copy link
Contributor Author

nope

Hmm, well that one crashes on cpython...but the test still passes somehow, weird.

@davidhewitt
Copy link

Uh oh; thanks for reporting! I'll start investigation now...

@davidhewitt
Copy link

davidhewitt commented Jul 4, 2021

pytest tests/hazmat/backends/test_openssl_memleak.py::TestAssertNoMemoryLeaks::test_errors

I think this one's not related to PyO3. It looks like I'm able to get this minimal repro to segfault on main:

from cryptography.hazmat.bindings._openssl import ffi, lib

@ffi.callback("void *(size_t, const char *, int)")
def malloc(size, path, line):
    ptr = lib.Cryptography_malloc_wrapper(size, path, line)
    return ptr

@ffi.callback("void *(void *, size_t, const char *, int)")
def realloc(ptr, size, path, line):
    new_ptr = lib.Cryptography_realloc_wrapper(ptr, size, path, line)
    return new_ptr

@ffi.callback("void(void *, const char *, int)")
def free(ptr, path, line):
    pass

result = lib.Cryptography_CRYPTO_set_mem_functions(malloc, realloc, free)
assert result == 1

# Trigger a bunch of initialization stuff.
from cryptography.hazmat.backends.openssl.backend import backend

raise ZeroDivisionError

Backtrace for that one suggests openssl is doing some cleanup in atexit:

#0  0x0000000000aa6080 in ?? ()
#1  0x00007ffff73a5159 in ?? () from /lib/x86_64-linux-gnu/libcrypto.so.1.1
#2  0x00007ffff73a542f in OPENSSL_cleanup () from /lib/x86_64-linux-gnu/libcrypto.so.1.1
#3  0x00007ffff7e0ba27 in __run_exit_handlers (status=1, listp=0x7ffff7fad718 <__exit_funcs>,
    run_list_atexit=run_list_atexit@entry=true, run_dtors=run_dtors@entry=true) at exit.c:108
#4  0x00007ffff7e0bbe0 in __GI_exit (status=<optimized out>) at exit.c:139
#5  0x00007ffff7de90ba in __libc_start_main (main=0x4eee60 <main>, argc=2, argv=0x7fffffffdb98,
    init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffdb88)
    at ../csu/libc-start.c:342
#6  0x00000000005f9ece in _start () at ../Objects/obmalloc.c:1233

Wild guess: it's running cleanup after the interpreter has finalized, but because of the installed memory functions it's invoking now-dead Python objects?

Anyway, I'll swap to look at the other test instead now...

@davidhewitt
Copy link

I can also repro the PyPy crash. This is the top of the backtrace:

(gdb) bt
#0  pyo3::ffi::cpython::listobject::PyList_SET_ITEM (op=0x1114940, i=0, v=0x10b7480) at /home/david/.cargo/registry/src/github.com-1ecc6299db9ec823/pyo3-0.14.0/src/ffi/cpython/listobject.rs:25
#1  0x00007ffff025bddb in pyo3::types::list::new_from_iter (elements=..., convert=...) at /home/david/.cargo/registry/src/github.com-1ecc6299db9ec823/pyo3-0.14.0/src/types/list.rs:27
#2  0x00007ffff025b8e6 in pyo3::types::list::<impl pyo3::conversion::IntoPy<pyo3::instance::Py<pyo3::types::any::PyAny>> for alloc::vec::Vec<T>>::into_py (self=..., py=...)
    at /home/david/.cargo/registry/src/github.com-1ecc6299db9ec823/pyo3-0.14.0/src/types/list.rs:205
#3  0x00007ffff0257622 in <T as pyo3::callback::IntoPyCallbackOutput<*mut pyo3::ffi::object::PyObject>>::convert (self=<error reading variable: Cannot access memory at address 0x0>, py=...)
    at /home/david/.cargo/registry/src/github.com-1ecc6299db9ec823/pyo3-0.14.0/src/callback.rs:60
#4  0x00007ffff0256bef in pyo3::callback::convert (py=..., value=...) at /home/david/.cargo/registry/src/github.com-1ecc6299db9ec823/pyo3-0.14.0/src/callback.rs:184
#5  0x00007ffff0288839 in cryptography_rust::asn1::<impl pyo3::class::impl_::PyClassDescriptors<cryptography_rust::asn1::TestCertificate> for pyo3::class::impl_::PyClassImplCollector<cryptography_rust::asn1::TestCertificate>>::py_class_descriptors::METHODS::__wrap::{{closure}} (_py=...) at src/asn1.rs:137

Looks like PyO3/pyo3#1664 is probably the cause of the breakage. I think PyPy does expose PyList_SET_ITEM symbols etc (i.e. they're not macros and the internal representation of the PyPy list is different). https://github.com/PyO3/pyo3/blob/main/src/ffi/cpython/listobject.rs needs to take this into account.

Gotta take the dog for a morning walk and then will prepare a patchfix when I get back! 🐶

@birkenfeld
Copy link

Can you try with PyO3/pyo3#1713, to make sure this fixes it?

@davidhewitt
Copy link

Locally it seems to fix it for me (though I only ran the failing test rather than the full CI). I think it's worth moving ahead with PyO3 0.14.1.

@jameshilliard
Copy link
Contributor Author

I think it's worth moving ahead with PyO3 0.14.1.

Yeah, are you planning to put that up as a hotfix release shortly?

@alex
Copy link
Member

alex commented Jul 4, 2021

Shall we do a test run here, with CI pointed at git?

@jameshilliard
Copy link
Contributor Author

Shall we do a test run here, with CI pointed at git?

Sure, not exactly sure how to actually do that, you should be able to push to my branch though.

@alex
Copy link
Member

alex commented Jul 4, 2021

Done!

@alex
Copy link
Member

alex commented Jul 4, 2021

Ok, we appear green here. This will be mergable once 0.14.1 is released. Thanks for everyone's work here!

@davidhewitt
Copy link

PyO3 0.14.1 has been released. Thanks again to all involved in this!

@jameshilliard
Copy link
Contributor Author

Looks like all the tests are passing.

@reaperhulk
Copy link
Member

New method call APIs! That x509 PR will need some rebasing after this…

@reaperhulk reaperhulk merged commit 120e804 into pyca:main Jul 4, 2021
@jameshilliard jameshilliard deleted the update-pyo3 branch July 4, 2021 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants