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

constructing exceptions in rust #203

Open
azjezz opened this issue Nov 26, 2022 · 11 comments
Open

constructing exceptions in rust #203

azjezz opened this issue Nov 26, 2022 · 11 comments

Comments

@azjezz
Copy link
Contributor

azjezz commented Nov 26, 2022

I have been struggling trying to implement the following:

<?php

final class FooException extends Exception {
  public static function from_bar(string $bar): self {
    return new self("Bar: " . $bar);
  }
}

I tried the following:

#[derive(Debug)]
#[php_class]
#[extends(ce::exception)]
pub struct FooException {}

#[php_impl]
impl FooException {
    pub fn from_bar() -> Zval {
        let zval: Zval = ZendClassObject::new(FooException {}).into_zval(false).unwrap();

        zval
    }
}

this results in an error when calling FooException::from_bar() ( trying to call non-static method statically ).

i tried making from_bar() into a function, but this also didn't work, as non of the exception properties were initialized, the exception will always not have a backtrace, ( file being "" , and line being 0 )

I initially tried to create the object and call it's constructor:

this one works, but it's the same as before, backtrace is completely missing, while it works perfectly for "normal" classes, it fails for Exceptions, i believe there's a zend function that i should call on the exception object before/after constructing it, but i can't find it ( yet ) in the ZEND API.

use ext_php_rs::error::Result;
use ext_php_rs::ffi;
use ext_php_rs::types::Zval;
use ext_php_rs::zend::ClassEntry;

pub fn construct(class: &ClassEntry, arguments: &[Zval]) -> Result<Zval> {
    let len = arguments.len();
    let class_ptr = class as *const _ as *mut _;
    let constructor_ptr = class.constructor;

    let object = unsafe {
        let zend_object = ffi::zend_objects_new(class_ptr);

        ffi::object_properties_init(zend_object, class_ptr);

        ffi::zend_call_known_function(
            constructor_ptr,
            zend_object,
            class_ptr,
            std::ptr::null_mut(),
            len as _,
            arguments.as_ptr() as _,
            std::ptr::null_mut(),
        );

        zend_object
            .as_mut()
            .expect("error: failed to allocate memory for object")
    };

    let mut result = Zval::new();
    result.set_object(object);

    Ok(result)
}


pub extern "C" fn from_bar(ex: &mut ExecuteData, retval: &mut Zval) {
    if ex.parser().parse().is_err() {
        return;
    }

    let ce = /* exception class entry */;
    let c = construct(ce, &["bar!.".into_zval(false).unwrap()]).unwrap();

    *retval = c;
}

So the solution i found, is the following, which is extremely slow:

macro_rules! throw {
    ($ce:expr, $message:expr) => {
        let e: ::ext_php_rs::exception::PhpException =
            ::ext_php_rs::exception::PhpException::new($message.to_string(), 0, $ce);

        e.throw()
            .expect(&format!("Failed to throw exception: {}", $message));
    };
}

pub extern "C" fn from_bar(ex: &mut ExecuteData, retval: &mut Zval) {
    if ex.parser().parse().is_err() {
        return;
    }

    let ce = /* exception class entry */;
    throw!(ce, "bar!");
    let object = ExecutorGlobals::take_exception().unwrap();

    retval.set_object(object.into_raw());
}

any help would be appreciated :)

@ju1ius
Copy link
Contributor

ju1ius commented Nov 26, 2022

Yeah, inheritance doesn't work ATM...

@azjezz
Copy link
Contributor Author

azjezz commented Nov 26, 2022

i don't think this is related to inheritance, the construct function above will call parent constructor if it's called from the ce constructor ( assuming the ce refers to a class that is in PHP, no ext-php-rs as it doesn't support calling parent constructors ).

however, exceptions are different from normal classes, they need more context ( file, line, trace ), as i said, throwing and catching works, zend will make sure to construct the exception correctly, but it is slow, we need to look at how zend constructs an exception, and basically do the same, so that PhpException wont' be just holding a reference to ce and message in the future, but rather a zend object, if you call new(msg, 0, ce) it will construct that class immediately itself without throwing it, and only throws it when .throw() is called.

this will allow us to be able to construct ext-php-rs exceptions without throwing them.

this is something that a lot of PHP libraries do, to give you an example:

https://github.com/azjezz/psl/blob/b239bb7f8b8feb52908ad6b82ab153b6ff8b6111/src/Psl/Channel/Exception/ClosedChannelException.php#L7-L34

while throwing immediately from forSending() works, and can be implemented in ext-php-rs, it is not the desired behavior, because in PSL those exceptions don't get thrown when constructed, they get sent over the event loop via a suspension (
https://github.com/azjezz/psl/blob/b239bb7f8b8feb52908ad6b82ab153b6ff8b6111/src/Psl/Channel/Internal/BoundedChannelState.php#L84-L99 ), which is later suspended and throws ( https://github.com/azjezz/psl/blob/2.2.x/src/Psl/Channel/Internal/BoundedSender.php#L42-L50 )

@azjezz
Copy link
Contributor Author

azjezz commented Nov 26, 2022

correction: the construct function above will probably fail if Foo class doesn't define a construct but it's parent Bar has one, we can do something like this to retrieve the ctor ( rusty-pseudo code ):

fn get_constructor() -> option {
  constructor = none;
  if ce.constructor == null {
    traits = ce.traits;
    for trait in traits {
      if trait.has_constructor() {
        constructor = trait.get_constructor();
        break;
      }
    }
  
    if constructor.is_none() {
       if ce.parent != null {
         // we do the same for the parent
         return get_constructor(ce.parent);
       }
    }
  } else {
    constructor = some(ce.constructor);
  }

  constructor 
}

see: https://3v4l.org/ga9Oq

@ju1ius
Copy link
Contributor

ju1ius commented Nov 27, 2022

none of the exception properties weren't initialized, the exception will always not have a backtrace

I think the reason why there's no backtrace is that you might be trying to construct an exception outside of a VM stack frame (when EG(current_execute_data) == null).
If that's the case, then this is expected since no VM stack frame => no trace.
The function that creates your exception instance needs to be called from PHP (at least that's my understanding).

@azjezz
Copy link
Contributor Author

azjezz commented Nov 27, 2022

no, that's not the issue, if there's no stack frame, we would get an error.

see: https://heap.space/xref/php-src/Zend/zend_exceptions.c?r=45e224cf#205

we are missing something else that i still can't figure out :)

@ju1ius
Copy link
Contributor

ju1ius commented Nov 27, 2022

You would have an error if the exception is thrown without a stack frame, not when constructing the exception object, see https://heap.space/xref/php-src/Zend/zend_exceptions.c?r=45e224cf#244.

@azjezz
Copy link
Contributor Author

azjezz commented Nov 27, 2022

throwing it doesn't result in an error, if you try the code above an call do throw from_bar() in PHP, it would throw the exception, but with no trace.

@ju1ius
Copy link
Contributor

ju1ius commented Nov 27, 2022

throwing it doesn't result in an error, if you try the code above an call do throw from_bar() in PHP, it would throw the exception, but with no trace.

Mmm okay... have you tried to assert that ExecutorGlobals::get().current_execute_data is not a null pointer at the time you create the exception instance?

@azjezz
Copy link
Contributor Author

azjezz commented Nov 27, 2022

no, but i don't think that would change anything, as in my test script, the file looks exactly like this:

<?php

function foo() {
    return Psl\Channel\Exception\ClosedChannelException::forSending();
}

function bar() {
    throw foo();
}

function baz() {
    bar();
}

baz();

meaning it is not not possible for another exception to be present.

@ju1ius
Copy link
Contributor

ju1ius commented Nov 27, 2022

There's a discord channel that is more appropriate for these kind of lengthy discussions: https://discord.com/channels/115233111977099271/829877152048349266

@azjezz
Copy link
Contributor Author

azjezz commented Nov 27, 2022

for anyone reading this thread, until this issue is resolved, here's how you can create your own exceptions:

use ext_php_rs::convert::IntoZvalDyn;
use ext_php_rs::error::Result;
use ext_php_rs::ffi;
use ext_php_rs::types::Zval;
use ext_php_rs::zend::ClassEntry;

pub unsafe fn make_exception(
    class: Option<&'static ClassEntry>,
    arguments: Vec<&dyn IntoZvalDyn>,
) -> Result<Zval> {
    let class = class.unwrap();
    let len = arguments.len();
    let arguments = arguments
        .into_iter()
        .map(|val| val.as_zval(false))
        .collect::<Result<Vec<_>>>()?
        .into_boxed_slice();

    let class_ptr = class as *const _ as *mut _;
    let constructor_ptr = class.constructor;
    let object = class.__bindgen_anon_2.create_object.unwrap()(class_ptr);

    ffi::zend_call_known_function(
        constructor_ptr,
        object,
        class_ptr,
        std::ptr::null_mut(),
        len as _,
        arguments.as_ptr() as _,
        std::ptr::null_mut(),
    );

    let object = object
        .as_mut()
        .expect("error: failed to allocate memory for object");

    let mut result = Zval::new();
    result.set_object(object);

    Ok(result)
}

Note: this is not 100% guaranteed to work, if you found an issues with it, let us know :)

let exception = unsafe { make_exception(MY_EXCEPTION_CE, vec![&message]).unwrap() };

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants