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

Dereference arel_table aliases between requests #18673

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@avit
Copy link
Contributor

avit commented Jan 24, 2015

Repeatedly calling Model.arel_table.alias pushed additional state into the memoized class variable that never got cleared.

Because Arel::Table has mutable state (which I assume is needed for alias tracking) the memoized instance should not persist across different threads.

Dereference arel_table aliases between requests
Repeatedly calling `Model.arel_table.alias` pushed additional state into
the memoized class variable that never got cleared.
@sgrif

This comment has been minimized.

Copy link
Member

sgrif commented Jan 24, 2015

arel_table is not part of the public API

@sgrif sgrif closed this Jan 24, 2015

@avit

This comment has been minimized.

Copy link
Contributor Author

avit commented Jan 24, 2015

The failing tests might be due to the class variable @arel_table getting referenced elsewhere, so this patch might be too naïve just yet.

@avit

This comment has been minimized.

Copy link
Contributor Author

avit commented Jan 24, 2015

That's too bad @sgrif I suspect arel_table gets used a lot whether it's "public" or not.

This is still a subtle vector for a memory leak, so I'd like to resolve it if we could...

I just had a look through the ActiveRecord source and arel_table.alias is only ever called once, in a test. Why then, does the arel_table.aliases array exist, since ActiveRecord has its own alias tracker and seemingly doesn't use that from Arel::Table? (Maybe patching Arel to make it immutable is the better fix?)

@avit

This comment has been minimized.

Copy link
Contributor Author

avit commented Jan 24, 2015

@rails rails locked and limited conversation to collaborators Jan 25, 2015

@rails rails unlocked this conversation Jan 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment