Uploaded image for project: 'Fedora Repository Project'
  1. Fedora Repository Project
  2. FCREPO-774

REST Servlet throws exception under access load.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Fedora 3.4
    • Fix Version/s: Fedora 3.4.2
    • Component/s: legacy - Fedora
    • Labels:
      None
    • Environment:
      Operating in Tomcat on a Red Hat 5 VM. Identified in 3.4 RC1
    • Roadmap Theme:
      Interface: REST/HTTP

      Description

      Two exceptions are thrown periodically while accessing Fedora via the REST Servlet. The system is operating under a moderate-high load but none of its resources are bottlenecked. Subsequent access operations function normally with the exceptions thrown at random but regular intervals. The VM is also under load with the ingest of digital object and very small numbers of managed datastreams. The events seem to correspond to 500 notifications in the client. There does not seem to be any effect to the ingest operations.

      The exceptions are found in the localhost log attached. The catalina log is attached for completeness.

        Attachments

          Issue Links

            Activity

            Hide
            blekinge Asger Askov Blekinge added a comment -

            This bug derive from the same source as 811. The MIMETypedStream is causing it. This time, the code in BaseRestResource fails

            From BaseRestResource

             protected Response buildResponse(MIMETypedStream result) throws Exception {
                    if (result.MIMEType.equalsIgnoreCase("application/fedora-redirect")) {
                        URI location = URI.create(IOUtils.toString(result.getStream()));
                        return Response.temporaryRedirect(location).build();
                    } else {
                        ResponseBuilder builder = Response.ok();
            
                        if (result.header != null) {
                            for (Property header : result.header) {
                                if (header.name != null
                                        && !(header.name.equalsIgnoreCase("transfer-encoding"))
                                        && !(header.name.equalsIgnoreCase("content-type"))) {
                                    builder.header(header.name, header.value);
                                }
                            }
                        }
            
                        if (!result.MIMEType.equals("")){
                            builder.type(result.MIMEType);
                        }
                        builder.entity(result.getStream());
                        return builder.build();
                    }
                }
            

            Look at the two final lines.
            builder.entity(result.getStream());
            return builder.build();

            This is the execution flow.
            1. Builder is given a reference to the inputstream from "result"
            2. Builder constructs a Response, with the reference to the inputstream
            3. Method returns, and result is no longer needed
            4. result is garbagecollected
            5. result closes the inputstream, due to the bug
            6. The framework tries to serialize the Response object to something. This fails, as the stream is not closed

            Show
            blekinge Asger Askov Blekinge added a comment - This bug derive from the same source as 811. The MIMETypedStream is causing it. This time, the code in BaseRestResource fails From BaseRestResource protected Response buildResponse(MIMETypedStream result) throws Exception { if (result.MIMEType.equalsIgnoreCase( "application/fedora-redirect" )) { URI location = URI.create(IOUtils.toString(result.getStream())); return Response.temporaryRedirect(location).build(); } else { ResponseBuilder builder = Response.ok(); if (result.header != null ) { for (Property header : result.header) { if (header.name != null && !(header.name.equalsIgnoreCase( "transfer-encoding" )) && !(header.name.equalsIgnoreCase( "content-type" ))) { builder.header(header.name, header.value); } } } if (!result.MIMEType.equals("")){ builder.type(result.MIMEType); } builder.entity(result.getStream()); return builder.build(); } } Look at the two final lines. builder.entity(result.getStream()); return builder.build(); This is the execution flow. 1. Builder is given a reference to the inputstream from "result" 2. Builder constructs a Response, with the reference to the inputstream 3. Method returns, and result is no longer needed 4. result is garbagecollected 5. result closes the inputstream, due to the bug 6. The framework tries to serialize the Response object to something. This fails, as the stream is not closed
            Hide
            blekinge Asger Askov Blekinge added a comment -

            And for good measure, the relevant parts of the MIMETypedStream

                private final boolean gotStream = false;
            
                /**
                 * Retrieves the underlying stream.
                 * Caller is responsible to close the stream,
                 * either by calling MIMETypedStream.close()
                 * or by calling close() on the stream.
                 *
                 * @return The byte stream
                 */
                public InputStream getStream() {
                    return stream;
                }
            
                /**
                 * Closes the underlying stream if it's not already closed.
                 *
                 * In the event of an error, a warning will be logged.
                 */
                public void close() {
                    if (this.stream != null) {
                        try {
                            this.stream.close();
                            this.stream = null;
                        } catch (IOException e) {
                            logger.warn("Error closing stream", e);
                        }
                    }
                }
            
                /**
                 * Ensures the underlying stream is closed at garbage-collection time
                 * if the stream has not been retrieved. If getStream() has been called
                 * the caller is responsible to close the stream.
                 *
                 * {@inheritDoc}
                 */
                @Override
                public void finalize() {
                    if(!gotStream) {
                        close();
                    }
                }
            

            Notice that gotStream is declared final, to guard against anyone ever setting it to true

            Show
            blekinge Asger Askov Blekinge added a comment - And for good measure, the relevant parts of the MIMETypedStream private final boolean gotStream = false ; /** * Retrieves the underlying stream. * Caller is responsible to close the stream, * either by calling MIMETypedStream.close() * or by calling close() on the stream. * * @ return The byte stream */ public InputStream getStream() { return stream; } /** * Closes the underlying stream if it's not already closed. * * In the event of an error, a warning will be logged. */ public void close() { if ( this .stream != null ) { try { this .stream.close(); this .stream = null ; } catch (IOException e) { logger.warn( "Error closing stream" , e); } } } /** * Ensures the underlying stream is closed at garbage-collection time * if the stream has not been retrieved. If getStream() has been called * the caller is responsible to close the stream. * * {@inheritDoc} */ @Override public void finalize() { if (!gotStream) { close(); } } Notice that gotStream is declared final, to guard against anyone ever setting it to true
            Show
            penthes Stephen Bayliss added a comment - Also see http://fedora-commons.1317035.n2.nabble.com/fcrepo-user-Occasional-random-FilterSetup-Can-t-do-next-doFilter-error-td5657344.html for another report of this.
            Hide
            penthes Stephen Bayliss added a comment -

            Jörg Panzer and Janna Wemekamp both confirmed that the fix committed for this have resolved the issue.

            Show
            penthes Stephen Bayliss added a comment - Jörg Panzer and Janna Wemekamp both confirmed that the fix committed for this have resolved the issue.

              People

              • Assignee:
                Unassigned
                Reporter:
                ddavis Daniel Davis
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: