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. UpdTransportBindTest.java
        2 kB
        Christian Posta
      2. AMQ-3359.patch
        2 kB
        Christian Posta
      3. AMQ-3359.patch
        0.7 kB
        Amir Malekpour

        Activity

        Amir Malekpour created issue -
        Amir Malekpour made changes -
        Field Original Value New Value
        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 but netstat shows another UDP port number. The reason is that as seen in the
        following block, the UdpTransport constructor does not read this.port from remoreLocation and only reads its address and leaves it to be zero. The solution is to add this line: this.port = remoteLocation.getPort();

        public UdpTransport(OpenWireFormat wireFormat, URI remoteLocation) throws UnknownHostException, IOException {
                this(wireFormat);
                this.targetAddress = createAddress(remoteLocation);
                description = remoteLocation.toString() + "@";
            }
        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 but netstat shows another UDP port number. The reason is that as seen in the
        following block, the UdpTransport constructor does not read this.port from remoreLocation and only reads its address and leaves the this.port to be zero. The solution is to add this line: this.port = remoteLocation.getPort();

        public UdpTransport(OpenWireFormat wireFormat, URI remoteLocation) throws UnknownHostException, IOException {
                this(wireFormat);
                this.targetAddress = createAddress(remoteLocation);
                description = remoteLocation.toString() + "@";
            }
        Amir Malekpour made changes -
        Attachment AMQ-3359.patch [ 12481592 ]
        Amir Malekpour made changes -
        Original Estimate 1m [ 60 ] 5m [ 300 ]
        Remaining Estimate 1m [ 60 ] 5m [ 300 ]
        Amir Malekpour made changes -
        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 but netstat shows another UDP port number. The reason is that as seen in the
        following block, the UdpTransport constructor does not read this.port from remoreLocation and only reads its address and leaves the this.port to be zero. The solution is to add this line: this.port = remoteLocation.getPort();

        public UdpTransport(OpenWireFormat wireFormat, URI remoteLocation) throws UnknownHostException, IOException {
                this(wireFormat);
                this.targetAddress = createAddress(remoteLocation);
                description = remoteLocation.toString() + "@";
            }
        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() + "@";
        }
        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.
        Timothy Bish made changes -
        Status Open [ 1 ] Closed [ 6 ]
        Resolution Not A Problem [ 8 ]
        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.
        Amir Malekpour made changes -
        Resolution Not A Problem [ 8 ]
        Status Closed [ 6 ] Reopened [ 4 ]
        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.
        Christian Posta made changes -
        Attachment AMQ-3359.patch [ 12509927 ]
        Christian Posta made changes -
        Attachment UpdTransportBindTest.java [ 12509928 ]
        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.
        Timothy Bish made changes -
        Status Reopened [ 4 ] Closed [ 6 ]
        Resolution Cannot Reproduce [ 5 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Closed Closed
        28d 21h 52m 1 Timothy Bish 05/Jul/11 15:10
        Closed Closed Reopened Reopened
        58d 20h 26m 1 Amir Malekpour 02/Sep/11 11:37
        Reopened Reopened Closed Closed
        160d 5h 1m 1 Timothy Bish 09/Feb/12 16:38

          People

          • Assignee:
            Unassigned
            Reporter:
            Amir Malekpour
          • Votes:
            1 Vote for this issue
            Watchers:
            2 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