Add support for throwing Exception objects

[joehoyle]

Currently we only support throwing exception class entries, i.e. stateless exceptions. Ideally we also want to support throwing a ZVal which has a ClassEntry that extends the Exception PHP class. This can be used to throw stateful exceptions which is not uncommon.

This PR is missing one piece: throw_object will currently drop the Zval when the function is completed, causing reference / null pointer errors. It seems ZVal::Drop doesn't actually free the zval currently, it just sets the type to NULL (which also breaks things.) Discussed briefly in https://discord.com/channels/115233111977099271/1025314959179120714 on how best to solve this, but I'm not totally clear still! Ideally I think we want throw_object to own the zval but not free it once the function returns.

Related: #203

Fixed: #137

[ptondereau]

@joehoyle thank you for this PR, the CI is failing for now. Could you please fix it before a in-depth review?

[ptondereau]

Sorry, I've merged #212 earlier, you can rebase into master to fix some clippy warning

[joehoyle]

@ptondereau ah, have you handled the merge, or do I need to rebase the branch?

[ptondereau]

@joehoyle totally handled sorry for the annoying

[joehoyle]

Ok cool, I think mostly what I need is input on how to handle the Zval getting dropped after zend_throw_exception_object is called.

[ptondereau]

I've tested your branch with a small example:

use ext_php_rs::convert::IntoZval;
use ext_php_rs::exception::throw_object;
use ext_php_rs::prelude::*;

#[derive(Debug)]
#[php_class]
#[extends(ext_php_rs::zend::ce::exception())]
pub struct JsException {
    #[prop(flags = ext_php_rs::flags::PropertyFlags::Public)]
    message: String,
    #[prop(flags = ext_php_rs::flags::PropertyFlags::Public)]
    code: i32,
    #[prop(flags = ext_php_rs::flags::PropertyFlags::Public)]
    file: String,
}

#[php_function]
fn test_throw() {
    let error = JsException {
        message: "A JS error occurred.".to_string(),
        code: 100,
        file: "index.js".to_string(),
    };
    throw_object(error.into_zval(true).unwrap()).unwrap();

    println!("dropped")
}

#[php_module]
pub fn get_module(module: ModuleBuilder) -> ModuleBuilder {
    module
}

And it returns:

❯ ../../php/php-debug-82/bin/php -dextension=target/debug/libext_test_php.so index.php                                       
dropped
[1]    105306 segmentation fault (core dumped)  ../../php/php-debug-82/bin/php -dextension=target/debug/libext_test_php.so 

[joehoyle]

@ptondereau yes, this is expected, and what i see too, as when the Zval is dropped, the type is updated to null, that's what I posted my question about on this PR.

[joehoyle]

@davidcole1340 perhaps you could advise here, I know we talked briefly about shallow_clone and allocating ZVals on the heap. I think the issue I'm running into there is that Zval sets it's self to null on drop().

[joelwurtz]

Does this zval will be correctly drop by php engine ?
If this is case maybe you can use https://doc.rust-lang.org/stable/std/mem/struct.ManuallyDrop.html so this zval is never dropped by rust ?

[ju1ius]

If this is case maybe you can use ManuallyDrop so this zval is never dropped by rust ?

Yes, it is indeed a cleaner solution, e.g.

pub fn throw_object(zval: Zval) {
    let mut zv = core::mem::ManuallyDrop::new(zval);
    unsafe {
        zend_throw_exception_object(core::ptr::addr_of_mut!(zv).cast())
    };
}

[joelwurtz]

It could also use Box like :

let zval_ptr = Box::into_raw(Box::new(zval)); // *mut zval;

as stated by rust, it will not drop the value if used like this (and also avoid using it latter since it will be consumed by this call)

[ju1ius]

It could also use Box like Box::into_raw(Box::new(zval))

Nope, zvals must always live on the stack (unless part of another heap-allocated struct).

Using a Box here just creates an unnecessary heap allocation. Plus you'll have to reclaim the box using Box::from_raw (otherwise that would be a memory leak).

Ref: https://www.phpinternalsbook.com/php7/zvals/memory_management.html#zval-memory-management

[joehoyle]

Thanks @ju1ius, I used that code, PR updated.

[danog]

These build times are killing me, I'd rather just merge a pending build and then fix formatting with a second PR on master tbh :)

[joehoyle]

@danog haha that's nothing! Try using github actions for Arm builds :P

[danog]

@joehoyle

Try using github actions for Arm builds :P

Github actions + QEMU is precisely the reason why I run all my CI builds on a raspberry pi at home with woodpecker :D