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

in vue-server-renderer, cached attributes are never released leading to big memory footprint #6332

Closed
czonechan opened this issue Aug 9, 2017 · 7 comments

Comments

@czonechan
Copy link

czonechan commented Aug 9, 2017

Version

2.4.2

Reproduction link

https://github.com/vuejs/vue/blob/dev/packages/vue-server-renderer/build.js#L372

Steps to reproduce

<div :data-id="id" :data-url=""url></div>

What is expected?

not cache escape attribute

What is actually happening?

attribute is cached


see https://github.com/vuejs/vue/blob/dev/packages/vue-server-renderer/build.js#L372

the attribute will be cached when escaped.

@czonechan czonechan changed the title vue-server-renderer use cache to render escape attribute will lead to memoery leak vue-server-renderer cached the escape attribute will lead to memoery leak Aug 9, 2017
@posva
Copy link
Member

posva commented Aug 9, 2017

Please follow the Issue Reporting Guidelines and provide a minimal JSFiddle or Github repository containing a set of reproducible steps that can lead to the behaviour you described. Make sure to boil down the problem as much as possible. Thanks!

@czonechan
Copy link
Author

czonechan commented Aug 9, 2017

I am sorry that cannot provide a minimal JSFiddle or Github repository .
because it's a SSR case, and i find it just by read the source code.
you can see the vue-server-renderer source code may be understand what i say.
if i save the data in attribute, it will make the cache get bigger and bigger.

----- vue-server-renderer source code -------

function cached (fn) {
  var cache = Object.create(null);
  return (function cachedFn (str) {
    var hit = cache[str];
    return hit || (cache[str] = fn(str))
  })
}

function escape (s) {
  return s.replace(/[<>"&]/g, escapeChar)
}

var cachedEscape = cached(escape);

function renderAttr (key, value) {
  if (isBooleanAttr(key)) {
    if (!isFalsyAttrValue(value)) {
      return (" " + key + "=\"" + key + "\"")
    }
  } else if (isEnumeratedAttr(key)) {
    return (" " + key + "=\"" + (isFalsyAttrValue(value) || value === 'false' ? 'false' : 'true') + "\"")
  } else if (!isFalsyAttrValue(value)) {
    return (" " + key + "=\"" + (typeof value === 'string' ? cachedEscape(value) : value) + "\"")
  }
  return ''
}

@posva
Copy link
Member

posva commented Aug 9, 2017

But why would it get bigger when using the same property?

@czonechan
Copy link
Author

the cachedEscape is cached the attributte value, the value is difference in every request.

@posva
Copy link
Member

posva commented Aug 15, 2017

Can you please develop what situation is exactly leading to the leak, please.
Maybe caching is just too naive for attributes

@LinusBorg
Copy link
Member

@posva This is not an issue for "normal", static attributes I think, but "leaks" when e.g. ids are used as / in attribute values:

The cache could eventually contain every single id from your database if you use the ids of database entries as attribute values, for example:

<div :data-whatever="item.id">

Over time, as users request pages for different items from your server, the cache would grow and grow, containing all item ids we have in your DB.

And that could potentially happen for any data from your DB, if used in element attributes.

So we might rather need an lruCache that's limited in size.

@posva
Copy link
Member

posva commented Aug 17, 2017

@LinusBorg That's what I was thinking, but the issue wasn't clear at the beginning 😆
The problem is not a leak, but a never released memory

edit: Hate that github no longer logs when you change the title 😞

@posva posva changed the title vue-server-renderer cached the escape attribute will lead to memoery leak in vue-server-renderer, cached attributes are never released leading to big memory footprint Aug 17, 2017
FranklinTesla pushed a commit to FranklinTesla/vue that referenced this issue Sep 5, 2017
ztlevi pushed a commit to ztlevi/vue that referenced this issue Feb 14, 2018
f2009 pushed a commit to f2009/vue that referenced this issue Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants