Skip to content
This repository has been archived by the owner on Apr 18, 2023. It is now read-only.

Fix response already committed error/corruption of response #26

Merged
merged 1 commit into from
Sep 24, 2018
Merged

Fix response already committed error/corruption of response #26

merged 1 commit into from
Sep 24, 2018

Conversation

jonescc
Copy link
Contributor

@jonescc jonescc commented Sep 13, 2018

This fixes Unidata/thredds#1052 and other associated issues.

When serving a GetMap request and the client connection is closed prior to completing the response, javax.imageio.stream.MemoryCacheImageInputStream was not being closed properly due to a ClientAbortException being thrown when trying write to and then flush/close the stream (at https://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/9d617cfd6717/src/share/classes/javax/imageio/ImageIO.java#l1580). This class has a finalizer inherited from one of its parent classes (https://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/9d617cfd6717/src/share/classes/javax/imageio/stream/ImageInputStreamImpl.java#l871) which closes the stream if its still open. Finalizers are invoked some time later by the finalizer thread (essentially a random time later). Closing MemoryCacheImageInputStream closes its wrapped output stream (in this case the ServletOutputStream). Tomcat reuses ServletOutputStream instances and so this output stream may be being used by another request and being flushed/closed inappropriately during that processing.

This is a known issue with these core java classes and tomcat (refer https://mail-archives.apache.org/mod_mbox/tomcat-users/201306.mbox/%[email protected]%3E and https://bugs.java.com/view_bug.do?bug_id=6299405) with the recommended solution included below.

We have been using this code in our production thredds instance now for almost 2 years with no issues and no "Yikes! Response is already committed (Heiko's bug?)" messages in the log.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@dopplershift
Copy link
Member

jachope has signed the CLA for the organization, so we can ignore the CLA check.

@lesserwhirls
Copy link
Collaborator

Excellent - thank you for this fix!

@lesserwhirls lesserwhirls merged commit e83bf42 into Unidata:master Sep 24, 2018
@jonescc jonescc deleted the fix-response-already-committed branch September 25, 2018 02:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Yikes! Response is already committed (Heiko's bug?).
4 participants