Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace ZiplineScope with @ZiplineEscaping #1463

Open
squarejesse opened this issue Nov 14, 2024 · 0 comments
Open

Replace ZiplineScope with @ZiplineEscaping #1463

squarejesse opened this issue Nov 14, 2024 · 0 comments
Milestone

Comments

@squarejesse
Copy link
Contributor

squarejesse commented Nov 14, 2024

ZiplineScope is Difficult to Reason About

The goal of ZiplineScope is to make it easy to clean up pass-by-reference services. I don’t think it’s successful at this objective. I also think it gets the lifecycles wrong.

Consider this sample:

interface PriceListener : ZiplineService {
  fun priceChanged(newPrice: Int)
}

interface PriceService : ZiplineService {
  fun watch(symbol: String, listener: PriceListener)
}

At its best, the ZiplineScope API will automatically close PriceListener instances when the PriceService that received them is closed. That looks like this:

class RealPriceService : PriceService, ZiplineScoped {
  override val scope = ZiplineScope()

  private val watches = mutableListOf<Watch>()

  fun watch(symbol: String, listener: PriceListener) {
    watches += Watch(symbol, listener)
  }

  fun close() {
    scope.close()
  }
}

The framework recognizes that the price service implements ZiplineScoped and adds the passed-in services to that scope. That saves the need to loop over the PriceListeners later.

@ZiplineEscaping

I propose an alternative design: change Zipline to close passed-in services when the called function completes, unless the receiver opts-in to close them itself with an annotation.

interface PriceService : ZiplineService {
  fun watch(symbol: String, @ZiplineEscaping listener: PriceListener)
}

Because services can be passed either directly or indirectly (as properties of other serialized objects), the semantics of @ZiplineEscaping is that everything encoded for that parameter must be closed manually. For example:

interface PriceService : ZiplineService {
  fun watchAll(@ZiplineEscaping symbols: Map<String, PriceListener>)
}

With @ZiplineEscaping here, none of the PriceListeners are closed when this function completes. If we dropped that annotation, they would be closed when watchAll() returns.

I’ve described this feature as a new annotation: “@ZiplineEscaping opts-out of automatic closing”. But the real behavior change is that everything without the annotation is automatically closed when the function completes, and @ZiplineEscaping gets you the old behavior.

‘When the Function Completes’

Suspending functions complete when their continuation runs, or when they’re canceled. Throwing functions complete when the exception is returned.

Migration

Automatically closing services that weren’t previously automatically closed is a severe behavior-incompatible change. I’ve tagged this issue as ’Zipline 2.0’ to address that.

Probably the least dangerous way to migrate is to annotate all parameters in all services with @ZiplineEscaping, and then remove them when we want that behavior. (Yikes.)

@squarejesse squarejesse added this to the 2.0 milestone Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant