Promises: resolve a new Promise

[d3nd3]

(function() {
  let p = new Promise((r)=>{
    console.log("resolved");
    r(new Promise((r2)=>{
      setTimeout(()=>{
        console.log("settled")
        r2();
      },2000);
    }));
  });

  p.then((v)=>{
    console.log("settledr1");
  });
})();

settledr1 is printed instantly

expected:

outer promise settles/fulfills upon inner promise being resolved.

---

(function() {
  let p = new Promise((r)=>{
    console.log("resolved");
    r(new Promise((r2)=>{
      setTimeout(()=>{
        console.log("settled")
        //r2();
      },2000);
    }));
  });

  p.then((v)=>{
    console.log("settledr1");
  });
})();

settledr1 is printed instantly

expected:

settledr1 should not be printed because r2 inner promise never resolves.

[gfwilliams]

Is that new behaviour? I always assumed that when you resolved a promise, you resolved with the value you wanted to return.

So if you resolve but pass another promise as the value that you're resolving, that then gets added to the chain?

[gfwilliams]

I can't face trying to fix this at the moment, so I've just made it throw an exception if you try and resolve with a promise so at least you know it's not doing what you expect.

[d3nd3]

I am not sure if its new or not. But its based on the distinction between resolve and fulfilled. Would you like me to try and fix it?

I don't mind implementing it.

[gfwilliams]

Thanks! Yes, it'd be great if you could fix it - but ideally the less code you end up changing in the process the better. Hopefully it can be done by just linking the new promise in in the right place

[d3nd3]

Hey, I am about to test some code regarding this, but I want to test on the emulator instead of flash to my watch so often. Do you have some tips how I can update the emulator to use the firmware I push via github?

Ah it seems I can just run the linux version? Because I am testing non-bangle feature.

[gfwilliams]

Ah it seems I can just run the linux version?

Yes, totally! That's the best way to do it. It has some promise tests built in already - and will be able to check for memory leaks too

[d3nd3]

I will intend to fix this bug too, its related to .then() not always returning a new promise, when it needs to because they can spawn independent chains. Do you prefer to do it in separate pull requests or all in one?

(function() {
  p1 = Promise.resolve();
  let p2 = p1.then(()=>{
    return new Promise((y,n)=>{
      setTimeout(()=>{
        console.log("hm");
        y("hm");
      },1000);
    });
  });
  let p3 = p1.then(()=>{
    return new Promise((y,n)=>{
      setTimeout(()=>console.log("3"),4000);
    });
  });
  //p2 resolves first, but both p2 and p3 fire.
  p2.then((y,n)=> {
    return new Promise((y,n)=>{
      setTimeout(()=>console.log("p2then1"),1000);
    });
  });
  p3.then((y,n)=> {
    return new Promise((y,n)=>{
      setTimeout(()=>console.log("p2then2"),1000);
    });
  });

  console.log(p2===p3);

})();

Node output:

false
hm
p2then1
3

Espruino output:

>true
>
hm
p2then1
p2then2
3

[gfwilliams]

It depends how entwined they are I guess.

If they're reasonably separate sets of changes then two PRs would be good so it's easier to track down if it turns out something breaks later

[d3nd3]

Ok, I will make separate ones for each issue I discover, (if) I find more.

[gfwilliams]

Thanks! Just a note but there's https://github.com/espruino/Espruino/issues/2227 which I think is the issue you described above

[d3nd3]

Thanks! Just a note but there's #2227 which I think is the issue you described above

Good find, yes that is the same thing and described well. I have an alternate implementation of promises as reference, they do not seem to use a chain like system at all.

Out of curiosity what was the reasoning for using obj.chain.chain.chain?

I have a feeling it can be achieved without chain, but don't stress yourself in remembering, I will try to take care of this.

[d3nd3]

I have fully researched how other implementations can have the correct behaviour without a chain. The secret is to passthrough the error/value, you do not need to find the catch. Since only either resolve or reject can be called on any promise, only one of the handlers will ever be called. So eg. you reject or throw/return an error in a .then , but you chain it further... The error now needs to meet with a .catch(), if the .then()'s inbetween this then() and the .catch() have not implemented an onRejected() handler, the error/value gets passed down via the reject(value) handlers. A bit like how a call stack returns values.

[d3nd3]

I have made some progress, had to change quite a lot of the code to get the behavior I wanted. However, my weakness stems from not understanding the purpose and correct usage of jsvUnLock and jsvLock.

Is there something I can research to understand it better, at the moment it seems I am unlocking everything as often as I can, without really understanding if I should be.

If there is ref counts, isn't that enough? Why are locks needed as well as ref counts? What are the situations where you keep a var locked over a longer period?

I think I need a complete Primer on garbage collection and locks.

[d3nd3]

https://github.com/espruino/Espruino/blob/255dbb036942c59c2e937d3c80c206d88586be79/src/jswrap_promise.c#L243C5-L243C28

Say for example here, we unlock the resolv and reject callback function objects, after the function call of the executor.

By unlocking them, does that mean they are free to be garbage collected? The situation when the executor uses a delayed/asynchronous call like setTimout(()=>resolve(),5000), the resolve function is unlocked because of our call to unlock yet its not garbage collected? because its refcount is still above 0?

Did passing the resolve function object into setTimeout increase its refcount?

If I want to keep an object protected from garbage collection over a larger number of C function calls, would I not call jsvUnLock until the condition which satisfies when I am happy for it to garbage collected?

I think there is a high chance that I accidently use the lock system in place of the ref system. If I understand how the ref system works, then I won't have to keep so many locks.

I would think that locks are to be used in scenarios when there is not a stable refcount? So if objectA was child of objectB, objectA would have a refcount>0, but objectB would require a lock because it is not a child of anything, and if objectB got gabagecollected, it would collect objectA in the process?

Mb I need to just play around with the gc() call in interpreter and study trace() more. A detailed description of this should be published somewhere to increase contribution efforts to Espruino, as I feel this is the greatest barrier.

So, if I instantly jsvUnLock() the newly created variable, only refcounts keep it alive. So I have to consider if the variable will have 0 ref counts as to whether I lock it, and sometimes I can instantly unlock it and rely upon the refcounts to clean it up? Just trying to get the use cases in my head better...

[gfwilliams]

Honestly, it's a bit of a concern to hear you say you've rewritten most of the code for this and then asking how to locks work :) It felt like really the changes for this should have been quite minor! Promises are quite a core bit of Espruino especially on Bangle.js so I really don't want this to get broken or to suddenly use lots of memory or lose performance.

On the lock front, I'm sure it was documented somewhere, but I've just tried to push a reasonably comprehensive guide to https://github.com/espruino/EspruinoDocs/blob/master/info/Internals.md#garbage-collection-reference-counts-and-locks (eventually it'll be in https://www.espruino.com/Internals)

But basically:

• you shouldn't have to worry about refs. Use the jsvAddChild/ObjectGet/Set/etc functions and it's handled.
• If you have a JsVar* it needs to be locked. It'll be locked when you get it, and when you're done with the pointer you need to jsvUnLock it - but you shouldn't be keeping locks between calls

[d3nd3]

If the stakes are that high, then we can make a slow release of it, fine-tuning it and reducing it down as much as possible. And testing it slowly before merging it. There is no hurry.

I'm just laying down the foundations of it for now, so that it behaves correctly still. But thanks for the information I requested, it will enable me to validate my usage of locks a bit better and thus have cleaner code and less work your end.