Is DataramSocket intended to be used half duplex?

General JUCE discussion
1

I’m asking because when a DatagramSocket is constructed, the connected flag is set to true. Which is fine, since you can go:

DatagramSocket s (0x8000);
s.read…

And read from all adapters to port 0x8000. But if you want to write, you need to call connect first, since connect constructs the sockaddr pointer that is used by the sendto call inside the write method. But if you call connect, it checks if the socket handle is valid and, if it is, closes it and opens a new one. But the new socket is no longer bound to port used in construction, so it isn’t suitable for reading anymore.

It seems you could rebind it, but that usage strikes me as odd:

DatagramSocket s (0x8000, true);
s.connect (“255.255.255.255”, 0x8000);
s.bind (0x8000); // seems odd…

s.write (somemsg, len);
s.read…

If the intent is half duplex usage, then it seems like there would need to be a way to set the SO_REUSEADDR socket option in order to implement quite a UDP broadcast type protocols, since you often broadcast and receive on the same port. Actually, there probably should be a way to set the flag anyway. If you are implementing a standard UDP protocol, you’d like to be able to coexist and communicate with other apps on the same machine using the protocol.

There is another issue with using INADDR_ANY for all datagram socket binds for writing, since that translates into the first interface, which can often not be the interface you need (it’s never the socket you need on, say, an iPhone). But one question at a time! I first want to make sure I understand the expected usage of the class to start.

Thanks in advance for any feedback/insight!

2

Agh, it’s too long since I wrote/used that class to be able to remember the details! Haven’t time to refresh my memory on it right now and give you a sensible answer, but any insights or suggestions on it are welcome!

3

My insights tend to get pretty tedious. But, FWIW, enclosed is how I modified juce_socket.cpp and juce_socket.h for my experiment (porting an existing applet from iOS to JUCE and running on all JUCE platforms).

As it happens, the existing applet speaks several UDP protocols (we do networking a lot). The stuff I set out to work around where:

  • Connect drops existing socket (binding is lost)
  • Port 0 was disallowed (system assigns)
  • Binding is always to IP_ANY
  • Address can’t be shared

In a broadcast protocol, like ArtNet or LP Net discovery, there can be several different apps listening on the same machine, so address sharing was a must. The binding to IP_ANY was also a must fix, since on an iPhone, you’ll never broadcast anything, since you won’t bind to the WiFi adapter by default, same with many multi adapter setups on Windows and Mac.

Port 0 was just a ‘nice’ thing to work out, since there are times you can let the system assign the port and that is just one more possible conflict you can ignore. The connect/rebind thing was sort of in the middle. In a simple case, I could work around, but it made some stuff complicated, and I like simple in the code at the top (hence my enjoyment of JUCE!)

I tried to stick with the original model as I could understand it, and maintain backwards compatibility. First, I added a flag to one of the DatagramSocket constructors so that address sharing can be enabled for the socket, and then defaulted it to off. Next I allowed a port of 0 to be passed to the bind function, and I added an optional parameter for the local address to bind to to the constructor and to the DatagramSocket bind (defaulting to IP_ANY, so the behavior is unchanged if not specified).

I didn’t want to pass a uint32 for the local address, so I made a IpAddress class, much like your MACAddress class, pretty much a simple container, with a static member for collecting all the IPv4 addresses from all the interfaces on the given platform. I didn’t bother with IPv6, because it is a non issue in UDP/broadcast applications. That stuff is virtually all same segment.

Those changes got me up and running. I was able to broadcast, negotiate, etc., but I found that populating the structures was a pain, since most the protocols use network byte ordering and htonl, etc. require the socket headers, which are platform specific, so I made another class, Socket, which some static helpers and a helpful constant or two, so that I could implement the basic protocols without including any extra headers in the app. If that was elsewhere in the framework, I’m sorry, I took a quick look, but it’s big and I’m new.

Then, for fun, I went ahead and added to more methods to DatagramSocket so that you can subscribe and drop from multicasts, which is a better way for broadcast protocols to work. I went ahead and tested it be implementing a CITP test, and interacted with a media server.

The code isn’t terribly conforming, sorry, that wasn’t my main goal. I didn’t blatantly disregard the existing style, but I didn’t agonize over it either. Also, since the Introjucer blows it away each run, I stuffed it all in the one .cpp/.h pair, instead of spreading it out more logically. On a couple of things, I would have liked to be more conforming, but just didn’t have time. For example, one of the three variations of IpAddress::findAllIpAddresses uses the old style BSD ioctl. To be really portable, you have to be prepared for a weird overrun case. I grabbed that from something I had done before and didn’t have time to go back and rework HeapBlock in and debug it.

I’ve tried the new classes and UDP protocols on top of them on Windows, Mac, iOS, and Android. My Linux box is currently toast, sorry. The only Linux part that concerns me is the findAllIpAddresses, that uses the new ioctl/SIG protocol. It should be identical to Android, but should… :wink:

I didn’t make any changes to the StreamingSocket side. You were already turning Nagle off (TCP_NODELAY) and allowing address sharing on listen. There are probably a couple of things I would tweak to better support simple server stuff, but it was fun keeping it in a nice simple model.

Understand, this isn’t production code. It is an experiment on the side (though I did some basic testing so I won’t be embarrassed when I make my pitch for single platform at work!) - so you won’t be getting any ‘why don’t you merge…’ pouts!

BUT, I just didn’t see a way to address normal UDP/Broadcast applications without something changing. If nothing else, the always binding to IP_ANY (the lowest numeric interface for sending) is killer. Not just on iPhone, but on multi adapter machines in general. I figured that it was at least tossing over the wall as food for thought for when and if you revisit Socket support.

I still owe you an applet with some minor cosmetics, and my notes on getting Android up, but this little side track into a JUCE port has sucked up all my available ‘side’ bandwidth.

Regards,
-jjf

4

Ok, this clearly deserves some proper attention, but I haven’t time to get my brain around it right now… Will try to look at it soon - if I don’t, please bump it to remind me!

5

JJ, is there any particular reason why you set the connected flag to true by default in the DatagramSocket constructors? Shouldn’t it be set by bindToPort() so that a socket instance can be closed, then reopened?

Appart from that, this seems like valid improvements of the udp support in Juce. Although, as you said, the code might need some rework (I especially stumbled upon the void* pointer that gets modified in an helper function… :twisted: )

6

Jules already had Datagram sockets connected by default. I kept that because it made sense to me, at least with the first form of the constructor.

Datagram ports are inherently connectionless. Let’s say that I’m listening for ‘Is Anyone there?’ poll messages on a multicast protocol.

I can create the DatagramSocket (common-port, true, true) (can broadcast, address can be reused.

At that point, I can start receiving from the socket and will get broadcast messages for that port. Or, I can put

socket.addMulticastMembership (IpAddress (“224.something”));

Then start reading, and get multicast broadcasts for that port.

A then start listening. What I did do was add setting the socket options to the constructor, which was not done before.

You only need to call connect to set an address for writing/broadcasting. And I tweaked connect so that it doesn’t completely tear down datagram sockets (which cost all binding). That way you can connect to IpAddress::broadcast, get a response, then set your send to for the identified host, and send again without losing memberships, etc.

I’ve actually added a few tweaks while finishing up my test project, all on the StreamingSocket side. For example, when the remote side closes the socket, attempts to read or write generate a SIGPIPE on iOS and Mac (and presumably Android and Linux). So I added:

#if defined (SO_NOSIGPIPE)
                && setsockopt (handle, SOL_SOCKET, SO_NOSIGPIPE, (const char*) &one, sizeof (one)) == 0
#endif

To the SocketHelpers::resetSocketOptions function (I know it looks odd, but it is in a very fancy return statement Jules already had).

I’ll post the updated version in a few days after we’ve tortured the experiment/sample app here.

7

P.S. Sorry if I’m missing your usage model. I didn’t muck with a listening model of some kind that was already there for Datagrams, so I didn’t deal with a case where a datagram isn’t already bound, that path could be weird.

Also, FWIW, I didn’t add any void* that I can think of, though there were a fair number any the existing code. That SocketHelpers class was existing.

8

Yeah sorry, I noticed afterwards that all the questionable code I saw was actually not from you but was already there before! My apologies.
It was just some questions about coherency, because the original class has a bunch of stuff I don’t understand:
[list][]waitForNextConnection() method which is useless for datagram (unless I missed the point here, which is possible)[/]
[]a “connected” flag which has no real meaning in udp[/]
[]one of the SocketHelpers functions modifies the inner member of the DatagramSocket class (this void** stuff)[/]
[] etc.[/][/list]
I thought you would have cleaned all up. But your modifications are already impressing. I’ve started playing with the multicast functionalities. Do the operating systems require different privileges to be able to multicast?

I guess that this file really needs some cleanup. Maybe using a template or something, as a lot of code is shared. Error handling is lousy as well. I had to hunt down the errno set by write() to notice that the address I entered was wrong…

9

Multicasting should work on all platforms. The only one I haven’t tried is Linux. Same with the findAllIpAddresses, it should work on all, but I haven’t yet tested Linux.

Initially, I thought about changing more, then opted to try to keep all the changes backwards compatible. Also, I figured that when/if Jules gets around to it, he’ll do a better job than me anyway!

Looking at the odd listen for, I think you are correct. Connected should be set by bind, then ‘connected’ means 'virtually connected/ready to use. I’ll make and test that change.

I did already go back and cleanup the realloc usage in the horrible old-school ioctl variant, and so far, so good. It’s actually a utility we use quite a bit ourselves, so I’ve got about 5 people using it on different platforms in place of the originals.

I’ll give it a few more days, then post the revision (and, of course, the utility source will be public - though the response has been strong enough that I can safely predict a commercial license in our near future too!)

I’m glad you’ve gotten some use out of it.

10

Any updates on this ? I'm in progress of doing a very lightweight UPnP discovery mechanism, but it needs UDP Multicast to work, i.e. I need to setup a listener on a Multicast IP address. I have not seen anyhing in DatagramSocket that could help with this ATM ?

11

Maybe I don't understand the problem, but I am doing discovery with UDP multicast. I simply instantiate the receiving DatagramSocket with the 'enableBroadcasting' parameter set to true. I do the same thing with the sender DatagramSocket, and everything has worked wonderfully. Two issues that may need attention are, a) making sure you bind to the right IP address on machines with multiple adapters, b) on Windows I could not use the full broadcast address '255.255.255.255', but instead had to use a local one x.x.x.255 (I don't have my notes with me, so I can't remember why at the moment).

12

A multicast needs the socket to be able to bind to an address like 239.255.255.250 (i.e. not x.x.x.255, which is the normal broadcast address for a given subnet), also, you need to join a multicast. In a broadcast, everyone bound to x.x.x.255 will get the UDP packet, but not in multicast.

Check the DatagramSocket implementation in this thread for details.

13

As I suspected, it was my lack of understanding of the problem space. I'll dig into the discussion more to get more clarity.

14

Hi Jules :) The suggested changes I think would benefit JUCE to be able to be used in UPnP stuff easily. Could you please look at including the changes as suggested by jfitzpat ?

15

Yeah, sorry, I hadn't been following this thread. Will take a look..

16

FWIW You put in my IpAddress stuff long ago - though I keep meaning to ask you for an additional function. ;-)

I've actually changed my Socket classes quite a bit since they were posted above. Above was just an effort to tweak existing code without breaking anything already using the classes. I'd be happy to post/send what we are using now. It's still not Jules Prose, but a lot cleaner. Now that I think about it, I owe you some libgit2 wrapper classes too.

Sorry, been doing a lot of travelling for work the last few months.

-jjf

17

Hi, jfitzpat. Just started incorporating the changes for multicast into current JUCE codebase, however I have a question about the localAddress parameter in DatagramSocket constructor. Is it needed ? Isn't it sufficient to just join the multicast with addMulticastMembership ?