ActiveMQ
  1. ActiveMQ
  2. AMQ-3359

UDP Transport connector listens on a random port number

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Cannot Reproduce
    • Affects Version/s: 5.4.0, 5.4.1, 5.4.2, 5.5.0
    • Fix Version/s: None
    • Component/s: Broker
    • Patch Info:
      Patch Available

      Description

      The broker listens on a random UDP port number instead of the one configure in the URI. The port number changes each time the broker is restarted. However, the management console indicates that the broker's listening on the configured port number while it is not the case (netstat shows another UDP port number). The reason is that (as seen in the following block) the UdpTransport constructor does not assign "this.port" from remoteLocation but only reads the address and leaves "this.port" to be zero. Subsequently, Java API picks any available port number when it is creating the DatagraSocket. The solution is to add this line: "this.port = remoteLocation.getPort();" to the following constructor as seen in the accompanying patch.

      public UdpTransport(OpenWireFormat wireFormat, URI remoteLocation) throws UnknownHostException, IOException

      { this(wireFormat); this.targetAddress = createAddress(remoteLocation); description = remoteLocation.toString() + "@"; }
      1. AMQ-3359.patch
        0.7 kB
        Amir Malekpour
      2. AMQ-3359.patch
        2 kB
        Christian Posta
      3. UpdTransportBindTest.java
        2 kB
        Christian Posta

        Activity

        Hide
        Timothy Bish added a comment -

        Looks like you might need to update the unit tests to handle this fix otherwise you get a bunch of port in use exceptions. Should also add some tests to ensure that the port value specified is used.

        Show
        Timothy Bish added a comment - Looks like you might need to update the unit tests to handle this fix otherwise you get a bunch of port in use exceptions. Should also add some tests to ensure that the port value specified is used.
        Hide
        Timothy Bish added a comment -

        After some further investigation this doesn't seem to be an issue. The broker doesn't call the constructor that's referenced in this issue when it creates its transport server, it calls the Constructor taking a wireFormat and port number to bind to. The constructor referenced uses the remoteLocation address to connect to the broker as expected.

        Show
        Timothy Bish added a comment - After some further investigation this doesn't seem to be an issue. The broker doesn't call the constructor that's referenced in this issue when it creates its transport server, it calls the Constructor taking a wireFormat and port number to bind to. The constructor referenced uses the remoteLocation address to connect to the broker as expected.
        Hide
        Timothy Bish added a comment -

        Some testing seems to indicate that this is working as expected. Reopen if you can provide a test case the demonstrates the problem.

        Show
        Timothy Bish added a comment - Some testing seems to indicate that this is working as expected. Reopen if you can provide a test case the demonstrates the problem.
        Hide
        Amir Malekpour added a comment - - edited

        Well, I have to disagree with you on this. I even resorted to "println" to make sure the method that is called IS the one I've mentioned above. (Even if it wasn't this is clearly a bug in that method to ignore the provided port number, so ignoring the buggy method on the basis that it's not called does not seem sound to me, though as I sad it is called).
        Now as for a case that proves the existence of the bug, just run the broker with UDP transport and use netstat to check if the broker is actually listening on the specified port. You'll see that it isn't. Honestly, if this is not a convincing case I don't know what is.

        Show
        Amir Malekpour added a comment - - edited Well, I have to disagree with you on this. I even resorted to "println" to make sure the method that is called IS the one I've mentioned above. (Even if it wasn't this is clearly a bug in that method to ignore the provided port number, so ignoring the buggy method on the basis that it's not called does not seem sound to me, though as I sad it is called). Now as for a case that proves the existence of the bug, just run the broker with UDP transport and use netstat to check if the broker is actually listening on the specified port. You'll see that it isn't. Honestly, if this is not a convincing case I don't know what is.
        Hide
        Amir Malekpour added a comment - - edited

        Well, I have to disagree with you on this. I even resorted to "println" to make sure the method that is called IS the one I've mentioned above. (Even if it wasn't this is clearly a bug in that method to ignore the provided port number, so ignoring the buggy method on the basis that it's not called does not seem sound to me, though as I sad it is called).
        Now as for a case that proves the existence of the bug, just run the broker with UDP transport and use netstat to check if the broker is actually listening on the specified port. You'll see that it isn't. Honestly, if this is not a convincing case I don't know what is.

        Show
        Amir Malekpour added a comment - - edited Well, I have to disagree with you on this. I even resorted to "println" to make sure the method that is called IS the one I've mentioned above. (Even if it wasn't this is clearly a bug in that method to ignore the provided port number, so ignoring the buggy method on the basis that it's not called does not seem sound to me, though as I sad it is called). Now as for a case that proves the existence of the bug, just run the broker with UDP transport and use netstat to check if the broker is actually listening on the specified port. You'll see that it isn't. Honestly, if this is not a convincing case I don't know what is.
        Hide
        Christian Posta added a comment - - edited

        Amir,
        The unit tests to which Tim refers do indeed break when applying the patch you created, as they are intended. As it appears, the different constructors in the UdpTransport class are intended for different uses. The constructor on which you made you changes is intended by the client transport creation. Which means, as you pointed out, there must be a bug because the server code is calling that constructor intended for the client. I have written a simple test to demonstrate the error, and the patch I added to the JIRA fixes the issue. Strangely, this issue's fix is related to the issue seen here: AMQ-3275. For that JIRA, a patch was committed to the trunk (http://svn.apache.org/viewvc?view=revision&revision=1089864), but it appears that patch was never merged in to any release versions. Tim, maybe you can see why that is. In any event, I've submitted my code anyway as a patch which adds comments and cleans up the code a little bit. See UdpTransportBindTest.java for a simple integration test that shows the connection broken and working before/after applying the patch.

        Show
        Christian Posta added a comment - - edited Amir, The unit tests to which Tim refers do indeed break when applying the patch you created, as they are intended. As it appears, the different constructors in the UdpTransport class are intended for different uses. The constructor on which you made you changes is intended by the client transport creation. Which means, as you pointed out, there must be a bug because the server code is calling that constructor intended for the client. I have written a simple test to demonstrate the error, and the patch I added to the JIRA fixes the issue. Strangely, this issue's fix is related to the issue seen here: AMQ-3275 . For that JIRA, a patch was committed to the trunk ( http://svn.apache.org/viewvc?view=revision&revision=1089864 ), but it appears that patch was never merged in to any release versions. Tim, maybe you can see why that is. In any event, I've submitted my code anyway as a patch which adds comments and cleans up the code a little bit. See UdpTransportBindTest.java for a simple integration test that shows the connection broken and working before/after applying the patch.
        Hide
        Christian Posta added a comment -

        sorry for the many updates and comments, but i have one more. my patch was written against the 5.4.3 tag

        Show
        Christian Posta added a comment - sorry for the many updates and comments, but i have one more. my patch was written against the 5.4.3 tag
        Hide
        Timothy Bish added a comment -

        Added the supplied test case to trunk, test passes.

        Show
        Timothy Bish added a comment - Added the supplied test case to trunk, test passes.

          People

          • Assignee:
            Unassigned
            Reporter:
            Amir Malekpour
          • Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 5m
              5m
              Remaining:
              Remaining Estimate - 5m
              5m
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development