From 3678268cd729abc5c116bcb764af29b0283feb4a Mon Sep 17 00:00:00 2001 From: Arman Bilge Date: Sat, 1 Apr 2023 22:15:05 +0000 Subject: [PATCH 1/4] `AtomicCell#get` should not semantically block --- docs/std/atomic-cell.md | 2 +- .../src/main/scala/cats/effect/std/AtomicCell.scala | 11 ++++------- .../test/scala/cats/effect/std/AtomicCellSpec.scala | 12 ++++++++++++ 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/docs/std/atomic-cell.md b/docs/std/atomic-cell.md index 0f852312ed..1fb7e3834d 100644 --- a/docs/std/atomic-cell.md +++ b/docs/std/atomic-cell.md @@ -6,7 +6,7 @@ title: Atomic Cell A synchronized, concurrent, mutable reference. Provides safe concurrent access and modification of its contents, by ensuring only one fiber -can operate on them at the time. Thus, **all** operations may semantically block the +can operate on them at the time. Thus, all operations except `get` may semantically block the calling fiber. ```scala mdoc:silent diff --git a/std/shared/src/main/scala/cats/effect/std/AtomicCell.scala b/std/shared/src/main/scala/cats/effect/std/AtomicCell.scala index e5332bdde6..50d1887eae 100644 --- a/std/shared/src/main/scala/cats/effect/std/AtomicCell.scala +++ b/std/shared/src/main/scala/cats/effect/std/AtomicCell.scala @@ -166,8 +166,7 @@ object AtomicCell { )( implicit F: Concurrent[F] ) extends AtomicCell[F, A] { - override def get: F[A] = - mutex.lock.surround(ref.get) + override def get: F[A] = ref.get override def set(a: A): F[Unit] = mutex.lock.surround(ref.set(a)) @@ -199,13 +198,11 @@ object AtomicCell { )( implicit F: Async[F] ) extends AtomicCell[F, A] { - private var cell: A = init + @volatile private var cell: A = init override def get: F[A] = - mutex.lock.surround { - F.delay { - cell - } + F.delay { + cell } override def set(a: A): F[Unit] = diff --git a/tests/shared/src/test/scala/cats/effect/std/AtomicCellSpec.scala b/tests/shared/src/test/scala/cats/effect/std/AtomicCellSpec.scala index b80283322c..6476b9de83 100644 --- a/tests/shared/src/test/scala/cats/effect/std/AtomicCellSpec.scala +++ b/tests/shared/src/test/scala/cats/effect/std/AtomicCellSpec.scala @@ -106,6 +106,18 @@ final class AtomicCellSpec extends BaseSpec { op must completeAs(true) } + + "get should not block during concurrent modification" in ticked { implicit ticker => + val op = for { + cell <- factory(0) + gate <- IO.deferred[Unit] + _ <- (gate.complete(()) *> cell.evalModify(_ => IO.never)).start + _ <- gate.get + r <- cell.get + } yield r == 0 + + op must completeAs(true) + } } } } From b07feec06f07515d027e936c6137f269c2109374 Mon Sep 17 00:00:00 2001 From: Arman Bilge Date: Sat, 1 Apr 2023 22:16:40 +0000 Subject: [PATCH 2/4] Doc updates --- docs/std/atomic-cell.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/std/atomic-cell.md b/docs/std/atomic-cell.md index 1fb7e3834d..afd476ac7b 100644 --- a/docs/std/atomic-cell.md +++ b/docs/std/atomic-cell.md @@ -22,15 +22,15 @@ abstract class AtomicCell[F[_], A] { ## Using `AtomicCell` -The `AtomicCell` can be treated as a combination of `Semaphore` and `Ref`: +The `AtomicCell` can be treated as a combination of `Mutex` and `Ref`: ```scala mdoc:reset:silent import cats.effect.{IO, Ref} -import cats.effect.std.Semaphore +import cats.effect.std.Mutex trait State -class Service(sem: Semaphore[IO], ref: Ref[IO, State]) { +class Service(mtx: Mutex[IO], ref: Ref[IO, State]) { def modify(f: State => IO[State]): IO[Unit] = - sem.permit.surround { + mtx.lock.surround { for { current <- ref.get next <- f(current) From 6f1b5e2a2e7e1516a1994a244402a89009a584cc Mon Sep 17 00:00:00 2001 From: Arman Bilge Date: Sat, 1 Apr 2023 22:37:20 +0000 Subject: [PATCH 3/4] Release `gate` in `evalModify` --- .../shared/src/test/scala/cats/effect/std/AtomicCellSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/shared/src/test/scala/cats/effect/std/AtomicCellSpec.scala b/tests/shared/src/test/scala/cats/effect/std/AtomicCellSpec.scala index 6476b9de83..ccca849288 100644 --- a/tests/shared/src/test/scala/cats/effect/std/AtomicCellSpec.scala +++ b/tests/shared/src/test/scala/cats/effect/std/AtomicCellSpec.scala @@ -111,7 +111,7 @@ final class AtomicCellSpec extends BaseSpec { val op = for { cell <- factory(0) gate <- IO.deferred[Unit] - _ <- (gate.complete(()) *> cell.evalModify(_ => IO.never)).start + _ <- cell.evalModify(_ => gate.complete(()) *> IO.never).start _ <- gate.get r <- cell.get } yield r == 0 From 7d18dc04549f505bf84955a654284f0a84abb898 Mon Sep 17 00:00:00 2001 From: Arman Bilge Date: Sat, 1 Apr 2023 22:38:05 +0000 Subject: [PATCH 4/4] Update scaladoc --- std/shared/src/main/scala/cats/effect/std/AtomicCell.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/std/shared/src/main/scala/cats/effect/std/AtomicCell.scala b/std/shared/src/main/scala/cats/effect/std/AtomicCell.scala index 50d1887eae..d36d9e0cb6 100644 --- a/std/shared/src/main/scala/cats/effect/std/AtomicCell.scala +++ b/std/shared/src/main/scala/cats/effect/std/AtomicCell.scala @@ -25,7 +25,7 @@ import cats.syntax.all._ * A synchronized, concurrent, mutable reference. * * Provides safe concurrent access and modification of its contents, by ensuring only one fiber - * can operate on them at the time. Thus, '''all''' operations may semantically block the + * can operate on them at the time. Thus, all operations except `get` may semantically block the * calling fiber. * * {{{