Cancel Promise pattern (no cancellable promises)

Naveen Chawla naveen.chwl at gmail.com
Mon Dec 4 06:12:51 UTC 2017


kai, 1. too much code to illustrate your point, 2. It's as much bad
practice to kick off async functions from a constructor as it is to kick
them off from any synchronous function. i.e., not at all. 3. I think these
GC worries are over the top. Yes, if you have promises being awaited that
never resolve, they will build up if you keep creating them. In this case,
the solution can perhaps be a timeout that rejects or resolves them. That's
it. I'm not sure this issue is any more complex than this. Somebody point
me to an example case that shows otherwise if you think I'm wrong.
Something isn't garbage collected if it's active, otherwise it is
eventually. Relax.

On Mon, 4 Dec 2017 at 10:17 kai zhu <kaizhu256 at gmail.com> wrote:

> @joe, its generally bad-practice and confusing to user, to include async
> initialization code inside a constructor.  you are better off using a
> class-factory with some kind of async callback-mechanism, which at the very
> least informs the user you’re doing an async-operation during construction
> (and preventing any gc while async-code inside the factory-function is
> still active).
>
> here's a real-world example of using a class-factory with callback, to
> asynchronously “construct” a persistent db-table from indexeddb
>
>
> https://kaizhu256.github.io/node-db-lite/build..beta..travis-ci.org/apidoc.html#apidoc.element.db-lite.dbTableCreateOne
>
> dbTableCreateOne = function (options, onError) {/*
>  * this function will create a dbTable with the given options
>  */    var self;
>     options = local.normalizeValue('dict', options);
>     // register dbTable
>     self = local.dbTableDict[options.name] =
>         local.dbTableDict[options.name] || new local._DbTable(options);
>     self.sortDefault = options.sortDefault ||
>         self.sortDefault ||
>         [{ fieldName: '_timeUpdated', isDescending: true }];
>     // remove idIndex
>     local.normalizeValue('list', options.idIndexRemoveList).forEach(function (idIndex) {
>         self.idIndexRemove(idIndex);
>     });
>     // create idIndex
>     local.normalizeValue('list', options.idIndexCreateList).forEach(function (idIndex) {
>         self.idIndexCreate(idIndex);
>     });
>     // upsert dbRow
>     self.crudSetManyById(options.dbRowList);
>     self.isLoaded = self.isLoaded || options.isLoaded;
>     if (!self.isLoaded) { // asynchronously load persistent dbTable from indexeddb if it exists
>         local.storageGetItem('dbTable.' + self.name + '.json', function (error, data) {
>             // validate no error occurred
>             local.assert(!error, error);
>             if (!self.isLoaded) {
>                 local.dbImport(data);
>             }
>             self.isLoaded = true;
>             local.setTimeoutOnError(onError, null, self);
>         });
>         return self;
>     }
>     return local.setTimeoutOnError(onError, null, self);
> }
>
> storageGetItem = function (key, onError) {/*
>  * this function will get the item with the given key from storage
>  */    defer({ action: 'getItem', key: key }, onError);
> }
>
> storageDefer = function (options, onError) {/*
>  * this function will defer options.action until storage is ready
>  */    var data, isDone, objectStore, onError2, request, tmp;
>     onError = onError || function (error) {
>         // validate no error occurred
>         local.assert(!error, error);
>     };
>     if (!storage) {
>         deferList.push(function () {
>             defer(options, onError);
>         });
>         init();
>         return;
>     }
>     switch (modeJs) {
>     case 'browser':
>         onError2 = function () {
>             /* istanbul ignore next */
>             if (isDone) {
>                 return;
>             }
>             isDone = true;
>             onError(
>                 request && (request.error || request.transaction.error),
>                 data || request.result || ''
>             );
>         };
>         switch (options.action) {
>         case 'clear':
>         case 'removeItem':
>         case 'setItem':
>             objectStore = storage
>                 .transaction(storageDir, 'readwrite')
>                 .objectStore(storageDir);
>             break;
>         default:
>             objectStore = storage
>                 .transaction(storageDir, 'readonly')
>                 .objectStore(storageDir);
>         }
>         switch (options.action) {
>         case 'clear':
>             request = objectStore.clear();
>             break;
>         case 'getItem':
>             request = objectStore.get(String(options.key));
>             break;
>         case 'keys':
>             data = [];
>             request = objectStore.openCursor();
>             request.onsuccess = function () {
>                 if (!request.result) {
>                     onError2();
>                     return;
>                 }
>                 data.push(request.result.key);
>                 request.result.continue();
>             };
>             break;
>         case 'length':
>             request = objectStore.count();
>             break;
>         case 'removeItem':
>             request = objectStore.delete(String(options.key));
>             break;
>         case 'setItem':
>             request = objectStore.put(options.value, String(options.key));
>             break;
>         }
>         ['onabort', 'onerror', 'onsuccess'].forEach(function (handler) {
>             request[handler] = request[handler] || onError2;
>         });
>         // debug request
>         local._debugStorageRequest = request;
>         break;
>     case 'node':
>         switch (options.action) {
>         case 'clear':
>             child_process.spawnSync('rm -f ' + storage + '/*', {
>                 shell: true,
>                 stdio: ['ignore', 1, 2]
>             });
>             setTimeout(onError);
>             break;
>         case 'getItem':
>             fs.readFile(
>                 storage + '/' + encodeURIComponent(String(options.key)),
>                 'utf8',
>                 // ignore error
>                 function (error, data) {
>                     onError(error && null, data || '');
>                 }
>             );
>             break;
>         case 'keys':
>             fs.readdir(storage, function (error, data) {
>                 onError(error, data && data.map(decodeURIComponent));
>             });
>             break;
>         case 'length':
>             fs.readdir(storage, function (error, data) {
>                 onError(error, data && data.length);
>             });
>             break;
>         case 'removeItem':
>             fs.unlink(
>                 storage + '/' + encodeURIComponent(String(options.key)),
>                 // ignore error
>                 function () {
>                     onError();
>                 }
>             );
>             break;
>         case 'setItem':
>             tmp = os.tmpdir() + '/' + Date.now() + Math.random();
>             // save to tmp
>             fs.writeFile(tmp, options.value, function (error) {
>                 // validate no error occurred
>                 local.assert(!error, error);
>                 // rename tmp to key
>                 fs.rename(
>                     tmp,
>                     storage + '/' + encodeURIComponent(String(options.key)),
>                     onError
>                 );
>             }); ...
>
> storageInit = function () {/*
>  * this function will init storage
>  */    var onError, request;
>     onError = function (error) {
>         // validate no error occurred
>         local.assert(!error, error);
>         if (modeJs === 'browser') {
>             storage = window[storageDir];
>         }
>         while (deferList.length) {
>             deferList.shift()();
>         }
>     };
>     if (modeJs === 'browser') {
>         storage = window[storageDir];
>     }
>     if (storage) {
>         onError();
>         return;
>     }
>     switch (modeJs) {
>     case 'browser':
>         // init indexedDB
>         try {
>             request = window.indexedDB.open(storageDir);
>             // debug request
>             local._debugStorageRequestIndexedDB = request;
>             request.onerror = onError;
>             request.onsuccess = function () {
>                 window[storageDir] = request.result;
>                 onError();
>             };
>             request.onupgradeneeded = function () {
>                 if (!request.result.objectStoreNames.contains(storageDir)) {
>                     request.result.createObjectStore(storageDir);
>                 }
>             };
>         } catch (ignore) {
>         }
>         break;
>     case 'node':
>         // mkdirp storage
>         storage = storageDir;
>         child_process.spawnSync(
>             'mkdir',
>             ['-p', storage],
>             { stdio: ['ignore', 1, 2] }
>         );
>         onError();
>         break;
>     }
> }
>
>
>
>
> On Nov 30, 2017, at 12:56 PM, Isiah Meadows <isiahmeadows at gmail.com>
> wrote:
>
>
>
> On Wed, Nov 29, 2017, 19:19 /#!/JoePea <joe at trusktr.io> wrote:
>
>> Hello all, I'm posting here because I'm thinking about how not to leak
>> memory, and wondering if I am.
>>
>> Suppose I have an object that is cleaned up (GC'd), and that object's
>> constructor (for example) ran an anonymous function that listened to a
>> Promise. For exampel:
>>
>> ```js
>> class Foo {
>>   constructor() {
>>     // ...
>>     this.somePromise.then(() => {console.log('this will never fire if
>> this object is GC'd')})
>>   }
>> }
>> ```
>>
>> Suppose that if this object gets collected, then it means that there is
>> nothing in my application that will ever be able to resolve this object's
>> `somePromise`.
>>
>
>> Does the engine also garbage collect `somePromise` and the attached
>> handler?
>>
>
> Yes, provided nothing else is referencing it.
>
>
>> Now, what if `somePromise` is referenced outside of this object by some
>> other code waiting on yet another promise who's handler will resolve this
>> object's `somePromise`? Does this prevent the object from being GC'd? Or is
>> the object GC'd, but `somePromise` is not and the above console.log should
>> still fire at some point (or memory will be leaked)?
>>
>
> The object itself is GC'd independently of its members absent a circular
> reference.
>
> A thing to keep in mind is that a promise could still be referenced by its
> resolving functions, even if the promise itself is no longer referenced by
> ECMAScript code. This keeps the promise state alive, so it can still fire
> (otherwise, you'd have observable GC). So, in this example, things will
> still fire regardless:
>
> ```js
> function foo() {
>     let p = new Promise(resolve => {
>         setTimeout(resolve, 1000)
>     })
>
>     p.then(() => {
>         console.log("This still fires")
>     })
>
>     // `p` is no longer referenced by the
>     // calling function, but it's still referenced
>     // by the enqueued `setTimeout` task, so
>     // it can't be collected
> }
>
> foo()
> ```
>
>
>>
>> */#!/*JoePea
>>
>> On Thu, Jan 12, 2017 at 1:12 PM, Isiah Meadows <isiahmeadows at gmail.com>
>> wrote:
>>
>>> And that's why we're waiting on the next meeting to happen with notes
>>> posted, so we can figure out what to do next. It's likely to get discussed,
>>> especially considering the current situation and pressing need for it.
>>>
>>> On Thu, Jan 12, 2017, 13:27 Jordan Harband <ljharb at gmail.com> wrote:
>>>
>>>> The Cancellable Promises proposal itself is currently withdrawn, but
>>>> don't forget that all of the previous discussion on cancellation in
>>>> promises led to that proposal.
>>>>
>>>> It would be shortsighted to pretend they don't exist, or that the spec
>>>> proposal won't matter forever to any other cancellation proposal, and doing
>>>> so won't help any alternative proposal.
>>>>
>>>> On Thu, Jan 12, 2017 at 7:44 AM, Jan-Ivar Bruaroey <jib at mozilla.com>
>>>> wrote:
>>>>
>>>> Cancellable promises is dead. Please don't hijack this thread
>>>> discussing them.
>>>>
>>>> Thanks,
>>>>
>>>> .: Jan-Ivar :.
>>>>
>>>>
>>>> _______________________________________________
>>>> es-discuss mailing list
>>>> es-discuss at mozilla.org
>>>> https://mail.mozilla.org/listinfo/es-discuss
>>>>
>>>>
>>>> _______________________________________________
>>>> es-discuss mailing list
>>>> es-discuss at mozilla.org
>>>> https://mail.mozilla.org/listinfo/es-discuss
>>>>
>>>
>>> _______________________________________________
>>> es-discuss mailing list
>>> es-discuss at mozilla.org
>>> https://mail.mozilla.org/listinfo/es-discuss
>>>
>>>
>> _______________________________________________
> es-discuss mailing list
> es-discuss at mozilla.org
> https://mail.mozilla.org/listinfo/es-discuss
>
>
> _______________________________________________
> es-discuss mailing list
> es-discuss at mozilla.org
> https://mail.mozilla.org/listinfo/es-discuss
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20171204/0388aae5/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Screen Shot 2017-12-04 at 11.44.14 AM.png
Type: image/png
Size: 316944 bytes
Desc: not available
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20171204/0388aae5/attachment-0002.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Screen Shot 2017-12-04 at 11.44.14 AM.png
Type: image/png
Size: 316944 bytes
Desc: not available
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20171204/0388aae5/attachment-0003.png>


More information about the es-discuss mailing list