Fix issue #4489

[durban]

This was obviously a can of worms, but I think I've finally fixed #4489. All three issues identified there:

1. A finalizer running "late". Two tests on this branch, timeout finalizer (start/release race) and racePair finalizer (start/release race) successfully reproduce this issue (on my machine). These tests are based on the reproducers by @armanbilge.
2. A finalizer not running at all. The test racePair finalizer variant (start/release race) here reproduces this issue successfully, the tests hangs (it is based on my own reproducer in the original issue).
3. The issue fixed by Make timeout/timeoutTo always return the outcome of the effect #4059. I've added two tests, propagate successful result from a completed effect and propagate error from a completed effect. These are variants of the tests originally added by Make timeout/timeoutTo always return the outcome of the effect #4059, but here specialized to Resource. (The approach of Resource runs its finalizers when timed out #4617 seems to make these tests fail/hang.)

---

First 2.: Resource#start previously changed the state in a Ref in 2 separate steps (a guarantee, then a flatMap). This leaves an opportunity for finalizeOuter to run between these 2 steps. The fix is to do the thing atomically (a single modify in a guaranteeCase). This fixes the hanging test specifically added for this issue.

Interestingly, the "finalizer running late" issue (1.) doesn't seem to be caused by the unintuitive behavior of Resource. (I remember believing last year that that was the cause, but I don't remember what convinced me of that.) Investigation now revealed, that the cause is... GenConcurrent#racePair. It previously didn't join the winner fiber; the winner fiber just completed a Deferred in a guaranteeCase. But what could possibly still run after that guaranteeCase? I'm glad you asked. (Narrator: "nobody asked.") The state-modifying code of Resource#start (mentioned in the previous paragraph). Due to racePair not joining the winner, that state-modfying code sometimes runs concurrently with anything after racePair. The fix is for racePair to join the winner. Technically, this change also fixes 2., not just 1. (because then finalizeOuter becomes ordered after the state-modification. But I still think it's good to do the other fix too; atomicity is good. (Obviously, changing a fundamental thing like racePair is risky, so I'm interested in thoughts about whether this will bite us somewhere else. But intuitively it seems logically correct to me, that racePair would join the winner.)

---

An open question: a #4059 regression test, as formulated by @djspiewak here does not pass with this branch. (I've added a variant here, timeout finalizer (#4059), which does pass.) The reason for that seems completely different: some existing code in Resource#start converts a Succeeded to a Canceled. I did not dare change that logic, as the whole finalizeOnComplete and confirmedFinalizeOnComplete seems very intentional. What I tried to do here was to preserve the existing logic, and make it tighter wrt cancellability and atomicity. (This is arguably a regression in this branch. Although, arguably, the onCancel is in the "wrong place" in that test.)

---

Besides these fixes, there is another one: an additional .uncancelable in Resource#start. I was not able to actually write a test that definitively shows that is necessary, but it seems to me that it could be a problem, so I've added it.

---

Note: this is against series/3.6.x, as #4489 was originally reported for that. I'm not sure if we still care about 3.6.x. If not, I can retarget this for 3.7.x. If yes, we should nevertheless merge this forward, and do a 3.7.1. (After proper review, of course.)

[durban]

Small update: the explanation I originally wrote for the "regression" is incorrect. I'm still investigating. (This is also the reason this is still a draft PR.)

[durban]

Update: I now believe the "regression" is not really a regression, but stems from the peculiar behavior of Resource.eval wrt cancellability. I think it just appears due to timing being different on this branch than on 3.6.x. (This behavior may very well be a bug, but it is definitely in existing code, and unrelated to the issues fixed here. I'll open a separate ticket.) With that, I think this PR should be ready for review.

---

For future reference, this is the test that fails on this branch:

    "timeout finalizer (#4059, onCancel)" in real {
      val test = IO.ref(false).flatMap { ref =>
        val program = Resource
          .eval(IO.uncancelable { poll =>
            ref.set(true) *> poll(IO.sleep(10.millis)).onCancel(ref.set(false))
          })
          .timeout(10.millis)
          .use_
        program.attempt.flatMap {
          case Left(_) =>
            ref.get.ifM(IO.raiseError(new Exception("not released")), IO.unit)
          case Right(_) =>
            IO.unit // no timeout
        }
      }

      test.parReplicateA_(10000).as(ok)
    }

This essentially the test here, just with the cases flipped (I think that's a mistake in the original), and uncancelable used tactically. It still fails. Resource.eval seems to have an extra cancellation point at the end.

[durban]

This is an unrelated change, but local case classes have a strange encoding with a LazyRef, so I've taken the opportunity to clean it up. (It's moved to the companion object.)

[fommil]

I think the test that is only testing Left should be testing both sides, no?

[durban]

You're right, done in 3b7ddb0.