Axis2
  1. Axis2
  2. AXIS2-4642

?wsdl query calls out.close() twice, confusing at least some Tomcat connectors

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.5.1
    • Fix Version/s: 1.5.4, 1.6.0
    • Component/s: kernel
    • Labels:
      None
    • Environment:
      Axis2.war deployed in Tomcat 6.0.24, using ajp to jk isapi_redirect to IIS on Windows Server 2008.

      Description

      AxisService.getWSDL() sends the response body for a ..service?wsdl query, then calls .flush() and .close() for its output stream. Upon return, ListingAgent.processListService() calls .flush() and .close() again for the same stream.

      The second close() interferes with the next request on the intermediate ajp connector socket.

      I don't know whether calling close() twice should be harmless but, in this case, it is not.

      [edit: Actually, the 2nd close() is harmelss. It's the flush() coming after the 1st close() that seems to be the problem.]

      1. Axis2Patch-4642-with-javadoc.txt
        8 kB
        Anjana Fernando
      2. Axis2Patch-4642.txt
        2 kB
        Anjana Fernando

        Activity

        Hide
        Anjana Fernando added a comment -

        Hi,

        I'm working on this.

        Cheers,
        Anjana.

        Show
        Anjana Fernando added a comment - Hi, I'm working on this. Cheers, Anjana.
        Hide
        Anjana Fernando added a comment -

        Hi,

        Hereby I'm attaching a patch to fix the issue.

        A small description and the rationale for the changes,

        I've made changes in "ListingAgent" and "AxisService" classes. In the AxisService class, methods which takes in "OutputStream" instances close the streams before they are returned, which is incorrect. As a general rule of thumb, you should not be closing streaming that you did not create. Because the one who's passing the output stream may have operations left to do with it. where in this case, the OutputStream is the servlet's response. Which we should let the servlet container to close, and we can just do "flush" operations to be sure that the data is written immediately. And in my opinion the flush operations also only need to be done in the method that is actually doing the writing, i.e. "getWSDL" in AxisService class.

        Cheers,
        Anjana.

        Show
        Anjana Fernando added a comment - Hi, Hereby I'm attaching a patch to fix the issue. A small description and the rationale for the changes, I've made changes in "ListingAgent" and "AxisService" classes. In the AxisService class, methods which takes in "OutputStream" instances close the streams before they are returned, which is incorrect. As a general rule of thumb, you should not be closing streaming that you did not create. Because the one who's passing the output stream may have operations left to do with it. where in this case, the OutputStream is the servlet's response. Which we should let the servlet container to close, and we can just do "flush" operations to be sure that the data is written immediately. And in my opinion the flush operations also only need to be done in the method that is actually doing the writing, i.e. "getWSDL" in AxisService class. Cheers, Anjana.
        Hide
        Andreas Veithen added a comment -

        I agree. However, we should complete the Javadoc of those methods so that they specify that it is the responsibility of the caller to close the stream. We should also check that these methods are not invoked by code (e.g. in java2wsdl) that implicitly relies on the fact that the stream is automatically closed.

        Show
        Andreas Veithen added a comment - I agree. However, we should complete the Javadoc of those methods so that they specify that it is the responsibility of the caller to close the stream. We should also check that these methods are not invoked by code (e.g. in java2wsdl) that implicitly relies on the fact that the stream is automatically closed.
        Hide
        Anjana Fernando added a comment -

        Hi Andreas,

        Here I've re-attached the patch by adding the suggested java doc comments. Also I've indeed earlier checked the usages of the methods I've changed to see if they are affected in any way. And as I found, there weren't any requirement to close the streams, the usages were mainly in a context where a servlet response object was used, where in that case, we anyway should not close the stream, and other places already had "flush" and "close" operations after the methods in question was called, which is actually what we are avoiding with this fix, to not to have close -> flush -> close situations.

        Regards,
        Anjana.

        Show
        Anjana Fernando added a comment - Hi Andreas, Here I've re-attached the patch by adding the suggested java doc comments. Also I've indeed earlier checked the usages of the methods I've changed to see if they are affected in any way. And as I found, there weren't any requirement to close the streams, the usages were mainly in a context where a servlet response object was used, where in that case, we anyway should not close the stream, and other places already had "flush" and "close" operations after the methods in question was called, which is actually what we are avoiding with this fix, to not to have close -> flush -> close situations. Regards, Anjana.
        Hide
        Amila Chinthaka Suriarachchi added a comment -

        applied the patch with revision 937237. Thanks Anjana for contribution.

        Show
        Amila Chinthaka Suriarachchi added a comment - applied the patch with revision 937237. Thanks Anjana for contribution.
        Hide
        Anjana Fernando added a comment -

        U're welcome ..

        Thanks,
        Anjana.

        Show
        Anjana Fernando added a comment - U're welcome .. Thanks, Anjana.

          People

          • Assignee:
            Unassigned
            Reporter:
            Bruce G Stewart
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development