Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using 100s of routes with ring async handlers causes StackOverflowError #187

Open
vincentjames501 opened this issue Mar 7, 2019 · 2 comments

Comments

@vincentjames501
Copy link

We have a large ring/compojure application with lots of api routes. We are trying to use ring's async handlers but we eventually run out of stack if a matching path is not found in time.

(let [handler (apply compojure/routes
                    (repeat 3000 (compojure/GET "/foo" [] {:status 200 :body "bar"})))]
  (handler {:uri "/foo2" :request-method :get}
           println 
           println))
=> java.lang.StackOverflowError: 

This is fine because it matches a route soon enough

(let [handler (apply compojure/routes
                    (repeat 3000 (compojure/GET "/foo" [] {:status 200 :body "bar"})))]
  (handler {:uri "/foo" :request-method :get}
           println 
           println))
=> {:status 200, :headers {}, :body bar}
=> nil

The issue is that it will create a new entry in the stack for each handler that doesn't match:

(defn routes
  "Create a Ring handler by combining several handlers into one."
  [& handlers]
  (fn
    ([request]
     (apply routing request handlers))
    ([request respond raise]
     (letfn [(f [handlers]
               (if (seq handlers)
                 (let [handler  (first handlers)
                       respond' #(if % (respond %) (f (rest handlers)))]
                   (handler request respond' raise))
                 (respond nil)))]
       (f handlers)))))

Even if you break it into chunks like so, you still get the same error.

(let [handler (apply compojure/routes
                     (map (fn [_] (apply compojure/routes (repeat 30 (compojure/GET "/foo" [] {:status 200 :body "bar"}))))
                          (range 100)))]
  (handler {:uri "/bar" :request-method :get}
           println 
           println))
=> java.lang.StackOverflowError: 

This can be rewritten using something like a go loop, but it's possible code is blocking and may unknowingly block peoples core async threads.

Any suggestions?

@vincentjames501
Copy link
Author

An idea (though not bullet proof), we could make wrap-route-matches return something synchronously like ::compojure/no-match and tweak routes like so?

(defn routes
  "Create a Ring handler by combining several handlers into one."
  [& handlers]
  (fn
    ([request]
     (apply compojure/routing request handlers))
    ([request respond raise]
     (letfn [(f [handlers]
               (loop [[handler & remaining-handlers] handlers]
                 (if handler
                   (let [respond' #(if % (respond %) (f remaining-handlers))]
                     (when (= (handler request respond' raise) ::compojure/no-route-found)
                       (recur remaining-handlers)))
                   (respond nil))))]
       (f handlers)))))

It can technically still run into the same issues if lots of routes match though the chances are slim.

@weavejester
Copy link
Owner

That's an interesting problem. As a temporary workaround, I'd suggest increasing the stack size for the running JVM, since you're not dealing with an unbounded stack.

Solving the problem in a more general way will require a bit of thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants