Dangerous shared objects on cache miss

[toto-castaldi]

No description provided.

[BryanDonovan]

Hi, thanks for this. I'm not sure I understand the need to clone though.

[toto-castaldi]

We had this situation :
two clients calling the same server at the same time with a cache miss. If one of the server logic modifies the cached data before returning it to the client also the second will found that change in the data

[BryanDonovan]

Seems like the client should do the clone operation since the cost of doing the clone in the cache manager might not be worth it for most people's situations.

[toto-castaldi]

Can be configurable in some way in order to maintain performance but my expected behavior is that I should not worry of other processes.
I'd say that this issue is only in the "wrap" section of the library, in the pure GET and SET this issue does not come up. So if is an optimization of the "wrap" should work.

[BryanDonovan]

I didn't think you could modify an object across processes. Are these clients running in the same process? In any case, it's possible to have multiple clients in the same process requesting the same data at the same time, so I'm ok with doing this if you make it configurable.

[BryanDonovan]

Can you use var, etc. here instead of ES6 syntax?

[BryanDonovan]

Please pin this version to exactly 1.5.0. I know it's a debatable choice :)

[coveralls]

Coverage decreased (-1.4%) to 97.902% when pulling 714e9ac on Skillbill:master into dc3924c on BryanDonovan:master.

[ankopainting]

We were bitten badly by this bug today.

eg.

let a = await myCache.wrap("string", () => {return {name: "frank", age: 6}});
console.log(a);
a.name = "bob"

let b = await myCache.wrap("string", () => {return {name: "frank", age: 6}});
console.log(b);

should return
{name: "frank", age: 6}
{name: "frank", age: 6}

before this change it returned
{name: "frank", age: 6}
{name: "bob", age: 6}

Please merge and release! :)

[BryanDonovan]

I'm looking into this now.

[BryanDonovan]

I didn't merge this before because it has a bug and no tests. I'm fixing the bug and adding tests now.

[BryanDonovan]

This is fixed in version 2.10.2

The issue was in the memory store only, so that's where I added the fix, which is to clone the object before setting it in the memory cache. See 84901cc