Bug 1014393

Thanks for the investigation Petr! As the root cause is clear, would you like provide a patch for this?

Just notice this is a bug in test codes. We don't ship test code in EAP, so this issue should reported upstream JIRA. I'll close this one and report it to JIRA and start to work on it.

This is a bug in test codes that we don't ship EAP, will fix it in upstream.

After some investigation on this problem seems more fixes need to be done outside the test code. Reopen it and work on it. Sorry for the chaos.

Weinan Li <weli> made a comment on jira RESTEASY-863

The problem is in closeReaders() method of XMLEntityManager. The good jdk one is:

{code}
 /**
     * Close all opened InputStreams and Readers opened by this parser.
     */
    public void closeReaders() {
        /** this call actually does nothing, readers are closed in the endEntity method
         * through the current entity.
         * The change seems to have happened during the jdk6 development with the
         * addition of StAX
        **/
    }
{code}

The problem one is:

{code}
    /**
     * Close all opened InputStreams and Readers opened by this parser.
     */
    public void closeReaders() {
        // close all readers
        for (int i = fReaderStack.size()-1; i >= 0; i--) {
            try {
                ((Reader)fReaderStack.pop()).close();
            } catch (IOException e) {
                // ignore
            }
        }
    }
{code}

see comment#1 solution recommended in [1]

Weinan Li <weli> made a comment on jira RESTEASY-863

Sequence diagram:

!https://issues.jboss.org/secure/attachment/12366704/RESTEASY-863-seq.png!

Class diagram:

!https://issues.jboss.org/secure/attachment/12366704/RESTEASY-863-class.png!

Weinan Li <weli> made a comment on jira RESTEASY-863

Sequence diagram:

!https://issues.jboss.org/secure/attachment/12366704/RESTEASY-863-seq.png!

Class diagram:

https://issues.jboss.org/secure/attachment/12366705/RESTEASY-863-class.png

Weinan Li <weli> made a comment on jira RESTEASY-863

Sequence diagram:

!https://issues.jboss.org/secure/attachment/12366704/RESTEASY-863-seq.png!

Class diagram:

!https://issues.jboss.org/secure/attachment/12366705/RESTEASY-863-class.png!

Weinan Li <weli> made a comment on jira RESTEASY-863

We need to override the close() method of "SelfExpandingBufferedInputStream", not sure whether this will affect other part. I'll do more investigation.

Weinan Li <weli> made a comment on jira RESTEASY-863

Here is the execute method in ApacheHttpClient4Executor:

{code}
 @SuppressWarnings("unchecked")
   public ClientResponse execute(ClientRequest request) throws Exception
   {
      String uri = request.getUri();
      final HttpRequestBase httpMethod = createHttpMethod(uri, request.getHttpMethod());
      try
      {
         loadHttpMethod(request, httpMethod);
   
         final HttpResponse res = httpClient.execute(httpMethod, httpContext);
   
         BaseClientResponse response = new BaseClientResponse(new BaseClientResponseStreamFactory()
         {
            InputStream stream;
   
            public InputStream getInputStream() throws IOException
            {
               if (stream == null)
               {
                  HttpEntity entity = res.getEntity();
                  if (entity == null) return null;
                  stream = new SelfExpandingBufferredInputStream(entity.getContent());
               }
               return stream;
            }
   
            public void performReleaseConnection()
            {
               // Apache Client 4 is stupid, You have to get the InputStream and close it if there is an entity
               // otherwise the connection is never released. There is, of course, no close() method on response
               // to make this easier.
               try
               {
                  if (stream != null)
                  {
                     stream.close();
                  }
                  else
                  {
                     InputStream is = getInputStream();
                     if (is != null)
                     {
                        is.close();
                     }
                  }
               }
               catch (Exception ignore)
               {
               }
            }
         }, this);
         response.setAttributes(request.getAttributes());
         response.setStatus(res.getStatusLine().getStatusCode());
         response.setHeaders(extractHeaders(res));
         response.setProviderFactory(request.getProviderFactory());
         return response;
      }
      finally
      {
         cleanUpAfterExecute(httpMethod);
      }
   }
{code}

The above method explicitly needs the close method of input stream:

{code}
try
{
   if (stream != null)
   {
      stream.close();
   }
   else
   {
      InputStream is = getInputStream();
      if (is != null)
      {
         is.close();
      }
   }
}
{code}

That means we cannot override the 'close' method of "SelfExpandingBufferedInputStream"

I've also checked the possible places that could configure "XMLEntityManager" not to close the stream, seems no configure could alter this behaviour.

(In reply to Petr Sakař from comment #7)
> see comment#1 solution recommended in [1]

Hi Petr, can we explicitly set the xercesImpl dependency used by RESTEasy in pom.xml to solve this problem?

Petr Sakař <psakar> made a comment on jira RESTEASY-863

Weinan,
have you tried solution proposed in https://bugzilla.redhat.com/show_bug.cgi?id=1014393#c1 or do you have some reasons why not to do it this way ?
Petr

(In reply to JBoss JIRA Server from comment #14)
> Petr Sakař <psakar> made a comment on jira RESTEASY-863
> 
> Weinan,
> have you tried solution proposed in
> https://bugzilla.redhat.com/show_bug.cgi?id=1014393#c1 or do you have some
> reasons why not to do it this way ?
> Petr

Could you please checke my investigations in above? I've explained the reason why we couldn't override the 'close' method of input stream. If you have any good suggestions, please let me know. I'm stilling trying to find a way to solve this...

Petr Sakař <psakar> made a comment on jira RESTEASY-863

Weinan,
very nice pictures. Can you please try this patch ?
Petr
{code:none}
diff --git a/resteasy-jaxrs/src/main/java/org/jboss/resteasy/core/interception/MessageBodyReaderContextImpl.java b/resteasy-jaxrs/src/main/java/org/jboss/resteasy/core/interception/MessageBodyReaderContextImpl.java
index 943c56c..4e12dae 100644
--- a/resteasy-jaxrs/src/main/java/org/jboss/resteasy/core/interception/MessageBodyReaderContextImpl.java
+++ b/resteasy-jaxrs/src/main/java/org/jboss/resteasy/core/interception/MessageBodyReaderContextImpl.java
@@ -9,6 +9,7 @@ import javax.ws.rs.core.MultivaluedMap;
 import javax.ws.rs.ext.MessageBodyReader;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.FilterInputStream;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Type;
 
@@ -102,7 +103,7 @@ public abstract class MessageBodyReaderContextImpl implements MessageBodyReaderC
    public Object proceed() throws IOException, WebApplicationException
    {
       if (interceptors == null || index >= interceptors.length)
-         return reader.readFrom(type, genericType, annotations, mediaType, headers, inputStream);
+         return reader.readFrom(type, genericType, annotations, mediaType, headers, new InputStreamWrapper(inputStream));
       try
       {
          return interceptors[index++].read(this);
@@ -112,4 +113,16 @@ public abstract class MessageBodyReaderContextImpl implements MessageBodyReaderC
          index--;
       }
    }
+   
+  private static class InputStreamWrapper extends FilterInputStream {
+
+    protected InputStreamWrapper(InputStream in) {
+      super(in);
+    }
+
+    @Override
+    public void close() throws IOException {
+    }
+
+  }
 }
{code}

Ron Sigal <ron.sigal> made a comment on jira RESTEASY-863

Off the top of my head, I'm wondering why not just catch the IOException if the close() fails and log a DEBUG message.

Ron Sigal <ron.sigal> made a comment on jira RESTEASY-863

I see.  I just found RESTEASY-456.

Ron Sigal <ron.sigal> made a comment on jira RESTEASY-863

Two comments.

1. According to http://mail-archives.apache.org/mod_mbox/xerces-j-users/200810.mbox/%3COFB0CA5E4A.8A179EB4-ON852574E4.0040887D-852574E4.004A0F5E@ca.ibm.com%3E, java.io.FilterInputStream.close() "does nothing", but I see

       public void close() throws IOException {
	in.close();
    }

in both the Oracle and IBM JDKs.  ???

2. Even though resetStream() is defined in org.jboss.resteasy.client.ClientResponse in the resteasy-jaxrs module, this issue seems to be specific to the JAXB provider.  In fact, resetStream() was added specifically in the fix for RESTEASY-456 "ClientResponse.getEntity(*) JaxB failure prevents future calls of getEntity(*) with other parameters: org.xml.sax.SAXParseException: Premature end of file."  So, rather than change some global behavior in the resteasy-jaxrs module, doesn't it make sense to fix the problem in the JAXB provider?  For example, the entityStream parameter in org.jboss.resteasy.plugins.providers.jaxb.AbstractJAXBProvider.readFrom() could be wrapped to make close() a NOP.  Actually, I'm wondering about the effect of doing that in the case of a normal (not caused by an exception) close.

Thoughts?

Weinan Li <weli> made a comment on jira RESTEASY-863

Thanks Petr & Ron! Things look clear now,  I'll try test your advices today :-)

Weinan Li <weli> made a comment on jira RESTEASY-863

Hi Petr, your patch is best :-)

I've done the local testings and finished the analysis. I'll create PR and submit it to github.

Weinan Li <weli> updated the status of jira RESTEASY-863 to Coding In Progress

Weinan Li <weli> updated the status of jira RESTEASY-863 to Open

Weinan Li <weli> updated the status of jira RESTEASY-863 to Resolved

Hi Weinan,

Since we're about to hit the 6.2 GA, to save time I'm hoping you can provide a draft release note in the Doc Text field above, as I'm unable to ascertain what the issue/resolution was in this ticket.

I think I understand that there was a problem when xercesImpl wasn't explicitly defined as a dependency, but as to how the issue manifested to users and how it was fixed, I'm at a loss.

Any help you can provide on this would be appreciated.

`Requires doc text` cleared as it is too late to include this ticket in the JBoss EAp 6.2.0 Release Notes.

This fix is not included in 6.2.0 yet.

should be in known issues, so requires documentation

Hi Scott, I have proposed a "Known Issue" doc in "Doc Text" in field if you need :-)

Fixed in RESTEasy 2.3.8

Verified in EAP 6.3.0.ER7.

Alessio Soldano <asoldano> updated the status of jira RESTEASY-863 to Closed