-
-
Notifications
You must be signed in to change notification settings - Fork 554
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
Creating a PHP wrapper example #470
Conversation
Hey there, @jgm I'm just pinging you because since this is an old PR, I know it's easy for you to lose track of it. |
This leaks memory just like the Python and Ruby wrappers, see #544. |
I'll add the |
Ideally these wrappers should define a function that can be imported and used in other code. (Perhaps in a long-running process.) The rb and py wrapper examples do that; your PHP example doesn't even try to do it, but it would be better to. In that case freeing the memory would be important. [Maybe? I don't know anything about PHP, so perhaps even in this case it would be freed automatically?] |
Yes, it would be freed automatically because the variable would be created inside that function and after the function's execution it leaves out of scope. But I'll still create the PR today adding the free call because it's better to have that explicitly so that people know that it's a resource that can/should be freed when used in a context where the variable will live. |
No, it wouldn't. You receive a char pointer from libcmark and the FFI implementation cannot know whether or how the data should eventually be freed. |
PHP's implementation of FFI makes that CData pointers free their resources once they go out of scope. |
The FFI implementation only frees "owned" data which was allocated by FFI itself. It cannot know about ownership of random pointers returned from C functions. Simply run the wrapper with valgrind to see the leak:
|
Yeah, I just saw the following line, which only cleans owned data: https://github.com/php/php-src/blob/ab589e4481f0cf35c8773e0c64dccc35b8870ae1/ext/ffi/ffi.c#L2392C19-L2392C22 |
Adding a PHP wrapper example using FFI.
Instead of using stdin, I chose to have a default markdown just to be more simple to execute the test. But that can be changed if preferred.