Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

Loss of multicast messages?

2008-02-27
2013-05-02
  • Hi,

    I think I discovered a major bug in the package org.cybergarage.upnp.ssdp:

    SSDPNotifySocket and SSDPSearchSocket both bind to the same address 239.255.255.250 and port 1900 (as it should be), but they both receive multicast packets from a different thread in the run() method. When they discover the packet is not useful for their purpose, they do nothing with this packet.

    We're testing in an environment with 10+ UPnP devices, and we have to wait untill a 'NOTIFY' messages for all different devices is received by the correct thread, so the devices get discovered/parsed correctly.

    Has this bug already been discovered, and is it already fixed or planned to? The relevant code in the SVN repo seems to be the same as ours, so I guess this is not the case.

    We're working a version that is about a half year old (with custom fixes to our needs...).

    regards,

    dvrslype

     
    • Ok,

      i see in the Java API in the MulticastSocket page:
      "When one sends a message to a multicast group, all subscribing recipients to that host and port receive the message."

      So my previous post should not be a bug, according to the java spec, but I definitely do not receive the same messages in the separate listening threads. We run this on an Ubuntu machine, with some sun 1.6 JVM.

      Anyone has a clue on this?

      regards,

      dvrslype

       
      • Stefano Lenzi
        Stefano Lenzi
        2008-02-28

        Hi dvrslype!

        I have few question and comment.

        Questions:
        1. Can you discover all the device?

        2. Are all device Cyberlink based?

        3. Are all device running inside in the same LAN? Can you provide some information on the network where you are testing the software?

        4. Are you complaining about response time or something else?

        Comments:

        1. From my understanding of the Cyberlink stack "bigger" issue is that the Thread which discover a new device(the Thread who receives a NOTIFY packet) has to create the new UPnPDevice, that means that the Thread has to:
        - retrieve the Device description by an HTTP request to the URL provided during the NOTIFY
        - parse the Device XML description
        - retrieve ALL the Service of the Device  by HTTP request to the URL provided in the device XML description
        - parse ALL the Service description
        the above operation are all time consuming, in particular the time required for the HTTP communication depends on the other peer(For instance: In our site we have some device on the network which can be discovered but the firewall between us and them drop all the HTTP packet so the Thread discovering the device has to wait the SocketTimeout). Thus, if the handling of packet from any UDP listening Thread requires a lot of time to complete, it is possible to lose some UDP packet. The simplest solution for that can be to use separate Thread for each UDP packet received.

        Ciao,
        Stefano "Kismet" Lenzi

         
        • Stefano,

          Q1: I assume we discover every device after a while.

          Q2: No, other devices are based on the Intel UPnP stack, uShare.

          Q3: Yes, just one LAN. Just a switch and one of us running DHCP.

          Q4: No, response times seem fair. Once we discovered the devices, we can call every device within a reasonable amount of time.

          this TODO message in SSDPNotifySocket raised my suspision:
          "//TODO Must be performed on a different thread in order to prevent UDP packet losses."

          Your conclusion is absolutely right: we lose packets indeed because the thread is doing other things. We fixed this temporarily by doing the "ctrlPoint.notifyReceived(packet);" in a seperate thread, as you suggested. Now all received packets are effectively run through notifyReceived, but it didn't fix the problem entirely, maybe we introduced some kind of synchronization issue by adding these threads, no much details for now.

          Is there a plan to fix this issue in the near future?

          dvrslype

           
          • Ok, we got it working as we want right now.
            Don't know if the code this fix introduces other problems...

            At first we just did the additional thread as mentioned before in SSDPNotifySocket:

            [code]
            if (ctrlPoint != null) {
              new Thread() {
                public void run() {
                  ctrlPoint.notifyReceived(packet);
                }
              }
            }
            [/code]

            off cource you have to set the ctrlPoint and packet variables final for this to work.

            This did not work as expected because the notifyReceived method calls the synchronized method addDevice(SSDPPacket ssdpPacket) which is getting the device descriptions and finds the device in the cyberlink structure. This may take long, so other notify packets have to wait to be processed. But we think the addDevice(SSDPPacket ssdpPacket) method should not be synchronized entirely, so we removed the synchronized keyword from the addDevice method, and added the synchronized keyword to the methods addDevice(Node rootNode), getDevice(Node rootNode) and getDevice(String name), which are called all 3 in addDevice(SSDPPacket ssdpPacket).

            This results in:

            [code]
            private void addDevice(SSDPPacket ssdpPacket){...}
            private synchronized Device getDevice(Node rootNode){...}
            public synchronized Device getDevice(String name){...}
            [/code]

            Can someone look into that, if this is correct. We don't get any bugs or anything from it right now, so we are happy :)

            dvrslype

             

            Related

            Code: code

            • Stefano Lenzi
              Stefano Lenzi
              2008-02-28

              I'm quite busy but I will try to look into it as soon as possible.

              Thank you for sharing your solution!

              Ciao,
              Stefano "Kismet" Lenzi