Data: Should not capture `headers` in the closure.

[nathanhammond]

In one place we capture the headers value inside of a closure it assumes that the value won't change before beforeSend is called. That is not a valid assumption.

https://github.com/emberjs/data/blob/f77dbf9382880b68e7985dba3b8235e09326de48/addon/adapters/rest.js#L1126-L1131

This should be modified to be captured inside of the beforeSend function which should therefore be bound every time.

[runspired]

Relying on side-effects (and even worse expecting side-effects) is not a good practice.

Options for a request should be complete by the time that ajaxOptions is finished. Either set headers before hand, or implement a custom ajaxOptions method.

[nathanhammond]

The problem is that Ember Data relies on jQuery's beforeSend to set headers. This does not play nicely if you instead use ember-ajax for ember-data, leveraging the ajax-support mixin.

Look at jQuery's documentation: http://api.jquery.com/jquery.ajax/

• headers is intended for baseline behavior.
• beforeSend is intended for late-binding and overrides.

I've watched three separate people get trolled by this (including today, which is why I came back to this issue). The problem comes from having beforeSend clobber the headers hash that ember-ajax produces (which happens after ember-data, and can be any kind of async).

I keep telling people to drop this into their base adapter (and a few other things into a modified Ajax service):

ajaxOptions() {
  const options = this._super(...arguments);
  delete options.beforeSend;

  const headers = this.get('headers');
  if (headers) {
    options.headers = headers;
  }

  return options;
}

As an aside, your response of "you're doing it wrong" speaks to an API weakness that should be addressed so that people fall into a pit of success rather than having to fight their tools. I'm obviously capable of debugging and working around this issue, but that is not necessarily the case for the majority of consumers (requires familiarity all the way down to the implementation of jQuery).

[snewcomer]

@allthesignals ref issue for container memory leaks

[snewcomer]

@runspired If you don't mind, can we reopen this issue? This + fastboot is the second app I've seen to hold onto the container and cause memory leaks.

https://github.com/ember-cli/ember-fetch/issues/38#issuecomment-489889471

[runspired]

@snewcomer nothing here or in the linked issue suggests that anything here holds onto the container.

[snewcomer]

I think there are 2 issues and both are likely disparate. The base adapter described above is something we have dropped into a few projects that use the ajax-support mixin.

Regarding the memory leak, headers as a computed was an issue.

The below change was how we fixed leaking the Ember container in FastBoot (which required rolling restarts to relieve the memory leak pressures).

From this:

headers: computed(function() {

to this in a base adapter:

get headers() {

I don't know the answer and only have a "best guess" as to why the above change fixed our memory issues. The computed seems to have leaked (the container, owner, Meta) to the hash options, which is why we are talking about these in the same issue.

@allthesignals Were you leveraging the ajax-support mixin and what ember-data version were you on? We were on ember-data 3.5.