steveklabnik / request_store

Per-request global storage for Rack.
https://github.com/steveklabnik/request_store
MIT License
1.48k stars 87 forks source link

Store should be cleared after each request #23

Closed afn closed 10 years ago

afn commented 10 years ago

In the interest of avoiding memory leaks, it would be prudent to clear the RequestStore not just before, but also after, each request.

Thanks! Tony

steveklabnik commented 10 years ago

Can you explain more about this to me, please?

afn commented 10 years ago

Currently, the middleware clears the request store first and then calls @app.call(env). This means that any objects stored in Thread.current[:request_store] will stay there until the middleware is called again. This can hamper garbage collection, because objects are reachable longer than necessary.

I think it would be preferable to clear the store after each request. That way, you're always guaranteed that the request store is empty both when RequestStore::Middleware.call is entered and when control leaves the method:

    def call(env)
      @app.call(env)
    ensure
      RequestStore.clear!
    end
steveklabnik commented 10 years ago

Ahhh, if we did it just after the request, then it'd have the same performance characteristics. Cool. I was thinking both before AND after, which seems excessive.

Do you want to write a patch so you get credit?

afn commented 10 years ago

See PR #25

wbotelhos commented 8 years ago

Hi,

Since this commit, I lose my asset_host value, so my image_url, for example, comes relative: From http://test.host/assets/image.png to /assets/image.png

If I put back RequestStore.clear! before @app.call(env), it works.