search

metosin / pohjavirta

Fast & Non-blocking Clojure wrapper for Undertow
169 stars 8 forks source link

Allow the response body to be an InputStream #12

Closed plexus closed 4 years ago

plexus commented 4 years ago

As per the Ring spec.

plexus commented 4 years ago

Also added java.io.File. Note that both of these are needed for reitit.ring/create-resource-handler, for serving from jars and the file system respectively.

plexus commented 4 years ago

I guess it was a little too easy :) I'll see what I can do.

plexus commented 4 years ago

BTW how do you handle the inlined java classes when running locally? do you just run javac on them?

plexus commented 4 years ago

Is 8k too small you think? I took this as inspiration: https://github.com/netroby/jdk9-dev/blob/master/jdk/src/java.base/share/classes/java/io/InputStream.java#L54

ikitommi commented 4 years ago

It's not the buffer size, but I think the code adds a new stack frame for every 8kb because of recursion? Sending 8Mb file would have a stack size of 1000, causing StackOverFlow?

plexus commented 4 years ago

@ikitommi have a look, I think we're there now. Thanks for spoon feeding me the solution :)

The only thing I'm not sure about is here:

  InputStream
  (send-body [stream ^HttpServerExchange exchange]
    (if (.isInIoThread exchange)
      (...)

      ;; getOutputStream assumes startBlocking has been called, i.e.
      ;; that (.isBlocking exchange). Does (.isInIoThread ex) always
      ;; imply (.isBlocking ex)?
      (with-open [stream stream]
        (io/copy stream (.getOutputStream exchange)))))
ikitommi commented 4 years ago

I'll test this and fine tune if something is needed. Good catch about the blocking, I think it needs to be set explicitely. Thanks!

plexus commented 4 years ago

@ikitommi any chance you could push a new alpha with these changes?

ikitommi commented 4 years ago

should be out now, with latest undertow, in alpha7.