How to use getSocket return value in PHP?

[kernelguy]

I would like to use Mosquitto-PHP together with React-PHP. React-PHP uses stream_select to detect if a socket is ready for something. The socket parameter used in stream_select and socket_select are resources, but the Mosquitto-PHP getSocket function returns a long integer.

I have never written any extension code my self, but tried to look at the code for stream and sockets, to see if I could change the unix socket to a PHP resource before returning from getSocket. But they seem to use two different resources. Does anybody know how to convert a unix socket (integer) to a PHP resource?

[mgdm]

I'm not entirely sure if this is useful, as I've not played with React and Mosquitto together yet, but I do have an example of using the result of getSocket with ext/event:

https://github.com/mgdm/Mosquitto-PHP/blob/master/examples/event.php

Does that help at all?

[kernelguy]

@mgdm Thank you very much. It turned out that I forgot to revert my attempt to return a resource from the getSocket function. :innocent:

But I learned about the pecl event extension, and found that React-PHP actually supports it already, I just had to uninstall the libevent extension first. After I found my bug in the getSocket function I reverted it back to libevent again and both seems to work fine. My previous test with socket_select and stream_select both failed, but somehow the libevent based extensions can handle the long integer file descriptor. That is good enough for me.

[mikini]

@kernelguy Did you try to specifically make a PHP socket resource and register it?

Looking at PHP's socket_create() a PHP socket resource is created from the integer reference returned from the POSIX socket() call. This is thrown into a struct with its type and is registered as a PHP resource and returned using;

RETURN_RES(zend_register_resource(php_sock, le_socket)); https://github.com/php/php-src/blob/master/ext/sockets/sockets.c#L1391 (slightly different in earlier branches https://github.com/php/php-src/blob/5.5.9/ext/sockets/sockets.c#L1391)

Surely something very similar must be doable on an already existing socket? Scope might be a problem, though. At least le_socket is gettable using php_sockets_le_socket() if that's in scope, but don't know about php_socket structure.

An alternative socket_create() could probably do it, but it would be annoying to have to deal with a homebuilt PHP compared to an extension.

Mikkel

[mgdm]

It doesn't return a resource right now, but when I get a spare minute I'll try it and see if it actually works.

[mikini]

Got it to compile using this patch. Haven't tested it runtime yet ;). I'm on Ubuntu 14.10 using PHP-5.5.12.

On master branch they use zend_register_resource() instead of ZEND_REGISTER_RESOURCE() in socket_create(). (valid link to non-master; https://github.com/php/php-src/blob/PHP-5.5.12/ext/sockets/sockets.c#L1370).

diff --git a/mosquitto.c b/mosquitto.c
index 7bb5a51..df25888 100644
--- a/mosquitto.c
+++ b/mosquitto.c
@@ -9,6 +9,7 @@
 #include "zend_API.h"
 #include "ext/standard/php_filestat.h"
 #include "ext/standard/info.h"
+#include "ext/sockets/php_sockets.h" 
 #include "php_mosquitto.h"

 zend_class_entry *mosquitto_ce_client;
@@ -602,7 +603,19 @@ PHP_METHOD(Mosquitto_Client, getSocket)
        PHP_MOSQUITTO_RESTORE_ERRORS();

        object = (mosquitto_client_object *) mosquitto_client_object_get(getThis() TSRMLS_CC);
-       RETURN_LONG(mosquitto_socket(object->client));
+       php_socket *php_sock = emalloc(sizeof *php_sock);
+       php_sock->bsd_socket = mosquitto_socket(object->client);
+       php_sock->type = AF_INET;
+       php_sock->error = 0;
+       php_sock->blocking = 1;
+       php_sock->zstream = NULL;
+
+    if (IS_INVALID_SOCKET(php_sock)) {
+        php_error_docref(NULL, E_WARNING, "Unable to create php_socket resource");
+        efree(php_sock);
+        RETURN_FALSE;
+    }
+       ZEND_REGISTER_RESOURCE(return_value, php_sock, php_sockets_le_socket());
 }
 /* }}} */

[mgdm]

Cool, thanks! If you get a chance to test it Is quite happily merge a PR :-)

On 4 Mar 2015, at 16:55, Mikkel Kirkgaard Nielsen notifications@github.com wrote:

Got it to compile using this patch. Haven't tested it runtime yet ;). I'm on Ubuntu 14.10 using PHP-5.5.12.

On master branch they use zend_register_resource() instead of ZEND_REGISTER_RESOURCE() in socket_create(). (valid link to non-master; https://github.com/php/php-src/blob/PHP-5.5.12/ext/sockets/sockets.c#L1370).

diff --git a/mosquitto.c b/mosquitto.c index 7bb5a51..df25888 100644 --- a/mosquitto.c +++ b/mosquitto.c @@ -9,6 +9,7 @@

include "zend_API.h"

include "ext/standard/php_filestat.h"

include "ext/standard/info.h"

+#include "ext/sockets/php_sockets.h"

include "php_mosquitto.h"

zend_class_entry *mosquitto_ce_client; @@ -602,7 +603,19 @@ PHP_METHOD(Mosquitto_Client, getSocket) PHP_MOSQUITTO_RESTORE_ERRORS();

    object = (mosquitto_client_object *) mosquitto_client_object_get(getThis() TSRMLS_CC);

• RETURN_LONG(mosquitto_socket(object->client));
• php_socket php_sock = emalloc(sizeof php_sock);
• php_sock->bsd_socket = mosquitto_socket(object->client);
• php_sock->type = AF_INET;
• php_sock->error = 0;
• php_sock->blocking = 1;
• php_sock->zstream = NULL; +
• if (IS_INVALID_SOCKET(php_sock)) {
• php_error_docref(NULL, E_WARNING, "Unable to create php_socket resource");
• efree(php_sock);
• RETURN_FALSE;
• }
• ZEND_REGISTER_RESOURCE(return_value, php_sock, php_sockets_le_socket()); } /* }}} */ — Reply to this email directly or view it on GitHub.

[kernelguy]

@mikini Nice digging ;-)

I tried it and got a runtime error on a missing IS_INVALID_SOCKET. After adding the following right after the include lines it works:

#ifdef PHP_WIN32
# include <win32/sockets.h> /* Not tested */
#else
# define IS_INVALID_SOCKET(a)   (a->bsd_socket < 0)
# define set_errno(a) (errno = a)
#endif

I looped the following test code:

            $read   = array($client->getSocket());
            $write  = null;
            $except = null;
            debug_log("print_r(\$read): ".print_r($read, true));
            $num_changed_sockets = socket_select($read, $write, $except, 30);
            if ($num_changed_sockets === false) {
            } else if ($num_changed_sockets > 0) {
                $client->loop(0);
            }

And the first call to getSocket returns a valid resource, but the second caused socket_select to bark an exception:

[04-Mar-2015 18:13:39 UTC] print_r($read): Array
(
[0] => Resource id #20
)

[04-Mar-2015 18:13:39 UTC] Polling mqtt loop
[04-Mar-2015 18:13:39 UTC] print_r($read): Array
(
[0] => Resource id #23
)

[04-Mar-2015 18:13:39 UTC] 
Exception: ErrorException
1 WARNING socket_select(): unable to select [9]: Bad file descriptor

[mgdm]

Hi folks, I made a PR to return resources instead, and added an example of its usage, in this PR: https://github.com/mgdm/Mosquitto-PHP/pull/23

If that's any use to you, then I'll merge it. I've got tests for it in another branch too that will land shortly.

[mikini]

Thanks for following up on this, Michael.

The PR looks fine, but one major issue came up after digging into @kernelguy 's loop problem; When the socket resource is out of scope the PHP garbage collector seems to deallocate the php_socket structure AND close the POSIX file descriptor inside it. As mosquitto_socket() will continue to return the (now closed) socket fd, subsequent use of any resource returned by getsocket() will fail with EBADF/"Bad file descriptor".

Output from PHP with "php_error_docref(NULL, E_NOTICE, "socket fd: %i", sockfd);" in getSocket() and var_dump($socket) in a loop doing assignment of getSocket() to $socket then socket_select():

Notice: Mosquitto\Client::getSocket(): socket fd: 26 in /web/https/mosquitto2.php on line 41
resource(2) of type (Socket)

Notice: Mosquitto\Client::getSocket(): socket fd: 26 in /web/https/mosquitto2.php on line 41
resource(3) of type (Socket) 
Warning: socket_select(): unable to select [9]: Bad file descriptor in /web/https/mosquitto2.php on line 49
socket_select() failed, reason: Bad file descriptor

Of course this also sever the mosquitto client library's connection with the MQTT broker. Can't elaborate on the correct way for the library to react to being back-stabbed like this. But at least I think mosquitto_socket() ought to check whether it has a valid socket before returning it. I'm cooking up a mosquitto bug report raising this question.

Regarding Mosquitto-PHP, could getSocket() somehow mark the socket resource to be non-collectable by the GC to prevent it from closing the mosquitto file descriptor?

Mikkel

[mgdm]

Ah ha. I have an idea, I'll attempt to implement it. Basically, we can up the ref count on the zval that getSocket() returns, so that it won't be GCed until the Mosquitto\Client gets GCed. It means I need to keep a handle to that zval inside the mosquitto_client_object structure so I can free it when the Client goes away.

[mikini]

Sounds exactly like "the right thing" to do ;). I've reported mosquitto bug 461714 (https://bugs.eclipse.org/bugs/show_bug.cgi?id=461714) regarding the validity of the mosquitto_socket() fd.

[kernelguy]

The ref count solution works for me.

I still have one issue though: When I close the mosquitto server, I get an onDisconnected event which is correct. Then when I start the server again, I get the on connected event, where I make a call to getSocket to get the resource to listen on incoming data in reactphp.

But now, using this socket, I get a lot of spurious "socket ready for read" events, even though no data is available. I tried to run some tests on the handle, like feof and stream_get_meta_data, but both fails because it is not a streaming resource.

Could it be that getSocket still returns the old socket?