Allow defining socket options

[skydiablo]

No description provided.

[skydiablo]

facepalm, any hints how to solve the PHP 5.3 problem? btw, is anyone using PHP 5.3 anymore???? :S please NOT!

[jsor]

The usual approach is, to make the method public and mark it @internal, for example like here:

https://github.com/reactphp/stream/blob/9f7c59c7550d080e9a016e2e722f6b3830763528/src/WritableResourceStream.php#L115-L116

[skydiablo]

thats ugly, sad...

[WyriHaximus]

It is :(

[WyriHaximus]

Is there a way to test this?

[skydiablo]

whats cracking?

[SimonFrings]

Hey @skydiablo, thanks for the ping 👍

Before going into detail by reviewing the files changed of this pull request, I want to take the time and talk about the "why" behind this pull request. What was the reason that you came across this and what problem does it solve. An answer to this can help me understand, if common use cases are in need of this feature, and will ultimately decide if this should be part of the project.

Interested in your input on this :)

[skydiablo]

my current project https://codeberg.org/volker.von.hoesslin/reactphp-dhcp-server needs this to define a interface.

[skydiablo]

@SimonFrings I really don't want to be rude or pushy, but do you need any more input? Can I offer further assistance in merging the PR?

[SimonFrings]

@skydiablo I used the time to talk with @clue about this pull request. The conversation above already touches on the impact with ext-sockets and my thoughts are: the more we rely on extensions, the less people can use this project.

I don't think this is a bad idea at all, but I'm not sure if reactphp/datagram is the right place to set these socket options. This decision always has an impact on the larger ecosystem, so perhaps it would make more sense to follow a similar solution as @clue found in https://github.com/clue/reactphp-multicast and set the socket options inside the project itself (looking at the createReceiver() method here).

What do you think about this approach?

[skydiablo]

Hmm, so the code is basically already built in a way that no dependencies are required. If the prerequisites are not met, the feature simply won't function. Alternatively, we could optionally pass a callback to the createServer or createClient function, which would receive the raw socket object for further external processing. Another option is to expose the raw socket object from the React\Datagram\Socket class using a getter.

[clue]

@skydiablo Thanks for filing this PR and sparking this discussion! I understand where you're coming from and agree that being able to set the low-level socket settings would be useful for a number of projects.

Your suggested changes appear reasonable on first sight, but I'm still not sure how I should feel about the fact that this extension may or may not be available and as a consequence the new API may or may not work. Besides the implementation, we'd also have to come up with a good documentation to describe this behavior and a full test suite that covers these code branches in detail. None of this would likely be required for your use case alone, but we'd also have to take the bigger impact on the ecosystem in mind here, so this is definitely something that needs to be covered as part of new feature suggestions like this.

I've discussed this feature before with @SimonFrings and we've come to the conclusion that a solution like https://github.com/clue/reactphp-multicast/blob/master/src/Factory.php#L106 suggested above might perhaps be a worthwhile alternative:

function create(): \React\Datagram\SocketInterface
{
    $stream = @stream_socket_server('udp://0.0.0.0:0', $errno, $errstr, STREAM_SERVER_BIND);
    if ($stream === false) {
        throw new RuntimeException('Unable to create socket: ' . $errstr, $errno);
    }
    
    $socket = socket_import_stream($stream);
    if ($stream === false) {
        throw new RuntimeException('Unable to access underlying socket resource');
    }
    
    // allow multiple processes to bind to the same address
    $ret = socket_set_option($socket, SOL_SOCKET, SO_REUSEADDR, 1);
    if ($ret === false) {
        throw new RuntimeException('Unable to enable SO_REUSEADDR');
    }
    
    return new \React\Datagram\Socket(Loop::get(), $stream);
}

Would be interested to hear your thoughts on this, is this something you've considered before?

[skydiablo]

hmmm, i can not see how will this help to my usecase? how can i assign me needed custom options, like:

array(
   SOL_SOCKET => array( 
     SO_REUSEADDR => 1,
     SO_BROADCAST' => 1,
     SO_BINDTODEVICE => 'eth0',
  ),
)

there is no chance to reconfig the raw-socket.

[clue]

@skydiablo I'm not sure I follow, the code above includes a socket_set_option($socket, SOL_SOCKET, SO_REUSEADDR, 1), so unless I'm missing something, this seems to be very similar what you're trying achieve? Have you tried similar logic for your socket options? What problem are you seeing exactly?

[skydiablo]

I need to set these two Options for my DHCP Server implentation:

SO_BROADCAST' => 1,
SO_BINDTODEVICE => 'eth0',

So, how can i assign this from the outside of this lib?

[skydiablo]

maybe just allow to access to the raw-socket in the \React\Datagram\Socket Class via a simple getter?

[SimonFrings]

I need to set these two Options for my DHCP Server implentation:

SO_BROADCAST' => 1,
SO_BINDTODEVICE => 'eth0',

So, how can i assign this from the outside of this lib?

@skydiablo From what I am getting out of the conversation with @clue I think you just have to do the following:

<?php

use React\Datagram\Socket as DatagramSocket;
use React\EventLoop\Loop;

class Foo
{
    function create(): \React\Datagram\SocketInterface
    {
        $stream = @stream_socket_server('udp://0.0.0.0:0', $errno, $errstr, STREAM_SERVER_BIND);
        if ($stream === false) {
            throw new RuntimeException('Unable to create socket: ' . $errstr, $errno);
        }
        
        $socket = socket_import_stream($stream);
        if ($stream === false) {
            throw new RuntimeException('Unable to access underlying socket resource');
        }

        $ret = socket_set_option($socket, SOL_SOCKET, SO_REUSEADDR, 1);
        if ($ret === false) {
            throw new RuntimeException('Unable to enable SO_REUSEADDR');
        }
        
        $ret = socket_set_option($socket, SOL_SOCKET, SO_BROADCAST, 1);
        if ($ret === false) {
            throw new RuntimeException('Unable to enable SO_BROADCAST');
        }

        $ret = socket_set_option($socket, SOL_SOCKET, SO_BINDTODEVICE, 'eth0');
        if ($ret === false) {
            throw new RuntimeException('Unable to enable SO_BINDTODEVICE');
        }
        
        return new DatagramSocket(Loop::get(), $stream);
    }
}

Have you already tried this out? If this isn't what you're looking for, perhaps it makes sense to get together in a quick Zoom call and go over this together with @clue sometime next week. We are currently in the midst of preparing the ReactPHP birthday, which will take place next Tuesday, so we won't be available until the 13th of July. WDYT?

[skydiablo]

Oh my God! I was so focused on the factory and didn't realize that I could use the constructor directly (facepalm). Thank you for the eye-opener. Of course, it would have been nice to be able to leverage the benefits of the factory, but I'll just write my own factory and be done with it! Thank you! And sorry for keeping you busy with the topic. Nevertheless, thank you for your work. I love ReactPHP, as you might have noticed. Now I'm trying to do everything with PHP, and it's working really well thanks to ReactPHP!

[SimonFrings]

Thank you! And sorry for keeping you busy with the topic.

@skydiablo You're welcome, and don't worry about it, we're always happy to help!

I love ReactPHP, as you might have noticed. Now I'm trying to do everything with PHP, and it's working really well thanks to ReactPHP!

Yes, I have noticed that ^^, glad you like the project and really appreciate the kind words ❤️