lampepfl / gears

A strawman for a low-level async library in Scala 3.
https://lampepfl.github.io/gears/
Apache License 2.0
247 stars 24 forks source link

Since transform(listeners) not preserve equality dropListener in transformed source not works. #88

Closed rssh closed 2 months ago

rssh commented 2 months ago

I'm not sure if this issue should be discussed; please convert this issue to a discussion if you think this is better.

https://github.com/lampepfl/gears/blob/d1b4a3edc8f52fd4ee1f4de445226d7cd800049a/shared/src/main/scala/async/Async.scala#L253C3-L254C41

      def dropListener(k: Listener[U]): Unit =
          src.dropListener(transform(k))

While reading code, I noticed the part mentioned. Given that listeners don't have equality and the need to guarantee that transform preserves reference equality, this will not work: it is possible that each transform call will generate a listener with a new identity. I am a bit curious about why Source has dropListener. If it's for resource-saving, perhaps considering other approaches, like weak references to the listener, would be beneficial.

Regards!

natsukagami commented 2 months ago

Right now ForwardingListener has a notion of equality by being a(n abstract) case class, and that was what would allow us to do such a thing.

dropListener is a purely resource saving operation, you're right (it is not guarantee to succeed - by being naturally racy with the source completing itself). The "correct" way to guarantee listeners cancellation is to write a locking listener that properly synchronizes itself and reject on being cancelled. With that in mind, we had a simple idea of just having dropListener compare by equality, and provide case-classed ForwardingListener so that they work with all Source-transformations by default. Perhaps we should rethink a bit about the equality requirements...

rssh commented 2 months ago

Thanks, understand now.