On 25/04/2019 21:16, Christopher Schultz wrote: > Mark, > > On 4/25/19 15:55, Mark Thomas wrote: >> On 23/04/2019 16:29, Olivier Jaquemet wrote: >>> On 23/04/2019 16:12, Christopher Schultz wrote: >>>> On 4/23/19 05:58, Olivier Jaquemet wrote: > >> <snip/> > >>>>> * Add the following directive to context.xml : <Resources >>>>> cachingAllowed="false" /> >>>> Okay. Why context.xml, by the way? >>> I don't even know (yet...) why this setting was added in the >>> first place in the environment where it was present... ! so why >>> this file... I don't know either :) > >> DefaultServlet is assuming caching is in place. If you disable it, >> you hit this issue. > >>>>> * Create a large file in the samples webapp, for example : >>>>> cd webapps/examples dd if=/dev/zero of=large.txt bs=1k >>>>> count=2000000 > >> <snip/> > >>>> Reading the code for FileResource.getContent, it's clear that >>>> the entire file is being loaded into memory, which obviously >>>> isn't going to work, here. I'm wondering why that's happening >>>> since streaming is the correct behavior when caching=false. >>>> Also strange is that DefaultServlet will attempt to call >>>> FileResource.getContent() -- which returns a byte[] -- and, if >>>> that returns null, it will call FileResource.getInputStream >>>> which ... calls this.getContent. So this looks like a >>>> special-case for FileResource just trying to implement that >>>> interface in the simplest way possible. > >> It is assuming it is working with a CachedResource instance rather >> than directly with a FileResource instance. > >>>> FileResource seems to implement in-memory caching whether it's >>>> enabled or not. >>>> >>>> I can't understand why this doesn't fail for the other kind of >>>> connector. Everything else is the same? You have two separate >>>> connectors in one instance, or are you changing the connector >>>> between tests? >>> >>> Everything is exactly the same as I have only one instance with >>> two separate connectors (AJP+HTTP). > >> I suspect HTTP avoids it because sendfile is enabled. > >> The DefaultServlet logic needs a little refactoring. > > And maybe FileResource, too? > > I wasn't able to follow the logic of whether caching or not caching > was enabled. I only did cursory checking, but it seemed like none of > the resources implementations included any caching-aware code at all. > Was I looking in the wrong place?
Don't think so. When caching is enabled everything gets wrapped in CachedResource. > If the resources are caching-aware, then I think the DefaultServlet > can just always use Resource.getInputStream. > > Hmm. That might cause a lot of unnecessary IO if the bytes are > actually available. That is a very tempting solution. The result is a LOT cleaner than the patch I just wrote. CachingResource is smart enough to cache the bytes and wrap them in a ByteArrayInputStream if Resource.getInputStream is called. My only concern is I think this introduces and additional copy of the data. I need to check that. > Maybe when caching is disabled, we need to wrap resources in an > UncachedResource object which always returns null from getContent() > and forces the use of an InputStream? My instinct is that would be too much but I'll keep it in mind in case I end up in a logic hole that that digs me out of. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
