Add a memoizedAcquire method to Resource

[satabin]

As mentioned in #3890 (comment), this is the PR taking over #3890 to add a specialized version of memoized for Resource.

Ref #3513

[satabin]

I did not forget about this PR. Testing it is currently blocked by the PureConc finalizer problem (see #3430). As soon as there is a fix, I will design a test using it.

[djspiewak]

Sadly I think this one might be really blocked on the PureConc stuff. It's hard to be totally comfortable with this absent tests.

[armanbilge]

To refresh my memory: the reason we couldn't write the test in IO (using the test runtime) is because I think that IO actually doesn't reproduce this bug thanks to the optimization introduced in #1742. Basically, the map is able to execute immediately without requiring an additional iteration of the runloop and thus skips the cancelation check.

[armanbilge]

It's hard to be totally comfortable with this absent tests.

Also point of clarification, there are several tests for memoizedAcquire (which is just a refactoring of memoize). During refactoring I introduced a bug, which Daniel U caught in review, and this test is intended to catch that.

[satabin]

It's hard to be totally comfortable with this absent tests.

Also point of clarification, there are several tests for memoizedAcquire (which is just a refactoring of memoize). During refactoring I introduced a bug, which Daniel U caught in review, and this test is intended to catch that.

Yes, the currently committed test does not reflect this, it is still an attempt I wrote before noticing I couldn't reproduce it with IO. I will update the test with what I think should work, once the PureConc behavior is fixed.

[djspiewak]

Alright, let's go for it.

[djspiewak]

Needs prePR

[satabin]

Needs prePR

Yes, I should have time to work on this this weekend.
Regarding the test I wanted to add to check that acquisition cancellation does not occur after the resource has been actually acquired and before it is put in the release list. Should I just not add it, or add and ignore it, until PureConc is fixed? What do you think is the best approach?

[djspiewak]

Regarding the test I wanted to add to check that acquisition cancellation does not occur after the resource has been actually acquired and before it is put in the release list. Should I just not add it, or add and ignore it, until PureConc is fixed? What do you think is the best approach?

Ideally add it and then mark pendingUntilFixed.

[armanbilge]

Instead of commenting the test you can use pendingUntilFixed :)

cats-effect/tests/js/src/test/scala/cats/effect/IOPlatformSpecification.scala

Lines 31 to 34 in 28f8f29

	"round trip through js.Promise" in ticked { implicit ticker =>
	forAll { (ioa: IO[Int]) =>
	ioa.eqv(IO.fromPromise(IO(ioa.unsafeToPromise())))
	}.pendingUntilFixed // "callback scheduling gets in the way here since Promise doesn't use TestContext"

[satabin]

True, I made a mistake in my test design. It should be ok now. See cd61033

[satabin]

@armanbilge I took your comment into account and added a pending test. Do you think this PR is ready to be merged now?