From d7cab0064d51c58fdc7b9b9002f90dec57df4105 Mon Sep 17 00:00:00 2001
From: Robert Jackson <me@rwjblue.com>
Date: Thu, 30 Jan 2020 19:42:06 -0500
Subject: [PATCH] Ensure adapters and serializers are destroyed upon store
 destruction. (#7020)

Prior to this change, adapters and serializers were never destroyed.
Commonly (and anecdotally) this is "fine" (heck this is the first report
of the issue in years!), but it is pretty common for adapters to do
async of their own (e.g. tracking adapter responses, exponential
backoffs, etc) and without `.destroy()` being called on them there is no
way for those adapters/serializer to ensure that async is canceled
appropriately.
---
 .../integration/store/adapter-for-test.js     | 30 +++++++++++++++++++
 .../integration/store/serializer-for-test.js  | 29 ++++++++++++++++++
 packages/adapter/addon/index.js               | 12 ++++++++
 packages/store/addon/-private/system/store.ts | 22 ++++++++++++--
 4 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/packages/-ember-data/tests/integration/store/adapter-for-test.js b/packages/-ember-data/tests/integration/store/adapter-for-test.js
index e62e9289fd6..79843f8d2e1 100644
--- a/packages/-ember-data/tests/integration/store/adapter-for-test.js
+++ b/packages/-ember-data/tests/integration/store/adapter-for-test.js
@@ -1,4 +1,7 @@
 import { setupTest } from 'ember-qunit';
+import { assign } from '@ember/polyfills';
+import { run } from '@ember/runloop';
+
 import { module, test } from 'qunit';
 import Store from 'ember-data/store';
 
@@ -297,4 +300,31 @@ module('integration/store - adapterFor', function(hooks) {
     );
     assert.ok(jsonApiAdapter === adapter, 'We fell back to the -json-api adapter instance for the per-type adapter');
   });
+
+  test('adapters are destroyed', async function(assert) {
+    let { owner } = this;
+    let didInstantiate = false;
+    let didDestroy = false;
+
+    class AppAdapter extends TestAdapter {
+      didInit() {
+        didInstantiate = true;
+      }
+
+      destroy() {
+        didDestroy = true;
+      }
+    }
+
+    owner.register('adapter:application', AppAdapter);
+
+    let adapter = store.adapterFor('application');
+
+    assert.ok(adapter instanceof AppAdapter, 'precond - We found the correct adapter');
+    assert.ok(didInstantiate, 'precond - We instantiated the adapter');
+
+    run(store, 'destroy');
+
+    assert.ok(didDestroy, 'adapter was destroyed');
+  });
 });
diff --git a/packages/-ember-data/tests/integration/store/serializer-for-test.js b/packages/-ember-data/tests/integration/store/serializer-for-test.js
index 58e01dde207..45ccae6aa93 100644
--- a/packages/-ember-data/tests/integration/store/serializer-for-test.js
+++ b/packages/-ember-data/tests/integration/store/serializer-for-test.js
@@ -1,4 +1,6 @@
 import { setupTest } from 'ember-qunit';
+import { run } from '@ember/runloop';
+
 import { module, test } from 'qunit';
 import Store from 'ember-data/store';
 
@@ -410,4 +412,31 @@ module('integration/store - serializerFor', function(hooks) {
       'We fell back to the -default serializer instance for the adapter defaultSerializer'
     );
   });
+
+  test('serializers are destroyed', async function(assert) {
+    let { owner } = this;
+    let didInstantiate = false;
+    let didDestroy = false;
+
+    class AppSerializer extends TestSerializer {
+      didInit() {
+        didInstantiate = true;
+      }
+
+      destroy() {
+        didDestroy = true;
+      }
+    }
+
+    owner.register('serializer:application', AppSerializer);
+
+    let serializer = store.serializerFor('application');
+
+    assert.ok(serializer instanceof AppSerializer, 'precond - We found the correct serializer');
+    assert.ok(didInstantiate, 'precond - We instantiated the serializer');
+
+    run(store, 'destroy');
+
+    assert.ok(didDestroy, 'serializer was destroyed');
+  });
 });
diff --git a/packages/adapter/addon/index.js b/packages/adapter/addon/index.js
index c7f1824eb5f..bfbc7d62d88 100644
--- a/packages/adapter/addon/index.js
+++ b/packages/adapter/addon/index.js
@@ -665,6 +665,18 @@ export default EmberObject.extend({
   shouldBackgroundReloadAll(store, snapshotRecordArray) {
     return true;
   },
+
+  /**
+    In some situations the adapter may need to perform cleanup when destroyed,
+    that cleanup can be done in `destroy`.
+
+    If not implemented, the store does not inform the adapter of destruction.
+
+    @method destroy [OPTIONAL]
+    @public
+    @optional
+  */
+  destroy: null
 });
 
 export { BuildURLMixin } from './-private';
diff --git a/packages/store/addon/-private/system/store.ts b/packages/store/addon/-private/system/store.ts
index 952335b39cd..7b02d82050b 100644
--- a/packages/store/addon/-private/system/store.ts
+++ b/packages/store/addon/-private/system/store.ts
@@ -2920,14 +2920,30 @@ const Store = Service.extend({
     return serializer;
   },
 
+  destroy() {
+    // enqueue destruction of any adapters/serializers we have created
+    for (let adapterName in this._adapterCache) {
+      let adapter = this._adapterCache[adapterName];
+      if (typeof adapter.destroy === 'function') {
+        adapter.destroy();
+      }
+    }
+
+    for (let serializerName in this._serializerCache) {
+      let serializer = this._serializerCache[serializerName];
+      if (typeof serializer.destroy === 'function') {
+        serializer.destroy();
+      }
+    }
+
+    return this._super();
+  },
+
   willDestroy() {
     this._super(...arguments);
     this._pushedInternalModels = null;
     this.recordArrayManager.destroy();
 
-    this._adapterCache = null;
-    this._serializerCache = null;
-
     this.unloadAll();
 
     if (DEBUG) {