I got a question regarding resources for you. In m...

# arrow

[Olaf Gottschalk]

I got a question regarding resources for you. In my use case, I have an application that has to start/manage/shutdown entities called Server - which is defined as an interface. So it looks along the lines of this:

interface Server {
  fun prepare()
  fun run()
  fun shutdown()
}

In order to properly handle these as resources, I have some fun execute() that does this:

fun execute() {
  resourceScope {
    servers.forEach {
      install({ it.prepare() }, { _, _ -> it.shutdown() })
    }
    servers.forEach(Server::run)
  }
}

Now, this assumes that all Server classes basically can do all of there setup in prepare and properly release all resources in shutdown.

In some servers though, I would like to also use the resource concept and install things in the same resource scope to allow them to live alongside the servers and properly get shutdown as well. Those types of server implementations typically do not define any code in their shutdown function, but they want to install more resources in the same resource scope.

That's why I extended my prepare fun like this:

interface Server {
  context(ResourceScope) fun prepare()
  fun run()
  fun shutdown()
}

To now execute my system, I need to manually bring the resource scope back into scope:

fun execute() {
  resourceScope {
    servers.forEach {
      install({ context(this@resourceScope) { it.prepare() } }, { _, _ -> it.shutdown() })
    }
    servers.forEach(Server::run)
  }
}

Now the big question: am I doing something wrong? Is it safe to hand in the same resource scope into the acquire step of a resource so this resource can also install resources in the same scope?

Note: the reason I do this is that I got several different types of Server implementations, some that need a "classic" prepare/shutdown step, others that are fine with installing resources!

[simon.vergauwen]

Hey @Olaf Gottschalk,
That is perfectly fine!

I think the AcquireStep became redundant, but we should be able to remove it in a binary compatible way.

[Youssef Shoaib [MOD]]

Yeah that's completely fine. I think we used to be afraid of that because the Scala lib this was based on didn't allow it, but it's fine.
In fact, if you use onRelease, your code can be way shorter:

fun execute() = resourceScope {
  servers.forEach {
    it.prepare()
    onRelease { _ -> it.shutdown() }
  }
  servers.forEach(Server::run)
}

This will do the correct release ordering as well btw (all components in prepare are registered first, then the server itself, and so the server will be released first, then its components will be)

[Olaf Gottschalk]

That was exactly what I feared - there was a comment about the AcquireStep being necessary to prevent some misuse. That got me worried. Could this AcquireStep maybe extend ResourceScope to allow an install within an acquire step without having to pull in the outer ResourceScope?

[Olaf Gottschalk]

Thanks for your answers! By looking at the code I thought this was all okay to do, but having your confirmation is really appreciated!

[Youssef Shoaib [MOD]]

Yeah I'm pretty sure it's not necessary at all anymore. In fact, install just runs the first lambda normally, then calls onRelease with the second, so registering things in the first lambda is equivalent to registering them before

[Youssef Shoaib [MOD]]

We could have it extend it and simply delegate to it yes. That's a pretty good idea actually!

[Youssef Shoaib [MOD]]

I would like to eventually remove AcquireStep, but as Simon said, we're locked in by binary compatibility (and maybe some source compatibility too?), but having it delegate is a nice middle ground.

[Alejandro Serrano.Mena]

(off-topic) I think in this case using fun ResourceScope.prepare() instead of using a context parameter would lead to a nicer API, and you would not need the intermediate context injection

[Youssef Shoaib [MOD]]

You can also use allocated and store the finalizer, and ensure it gets called on shutdown.

[Youssef Shoaib [MOD]]

I've thought about this a bit longer, and I think this might be a better design here:

interface Server {
  context(_: ResourceScope) fun prepare()
  fun run()
}
fun execute() = resourceScope {
  servers.forEach { it.prepare() } // maybe can be written as a method reference, but I've sometimes had weird compilation issues; I believe that's how it's supposed to work tho in that it binds the contexts immediately 
  servers.forEach(Server::run)
}

And the server is responsible to add its hooks in prepare, and so it can add a singular hook equivalent to shutdown, or it can add whatever components it wants that themselves add hooks. The interface is minimal (I don't think it can be made smaller because you want to register them all first, and then run, so the only smaller one is something like context(_: ResourceScope) fun prepare(): () -> Unit, which has the nice property that run can't be called before prepare, but it's a little overkill and needs a map)

The important question though is do you really need to have this as a concern in the interface? You can have the smart constructors take in ResourceScope directly. Server would then just have run as its only method. The only reason to design it in the way you have it is if the servers need to exist outside of a ResourceScope, but that seems very very unlikely. In fact, the smart constructor route is basically that single-function prepare thing I mentioned above, but more idiomatic and open-ended

[Olaf Gottschalk]

@Alejandro Serrano.Mena you're completely right. I recently moved to context parameters everywhere and I try to stick to the question how the function signature reads... So, in my case, I am not preparing a ResourceScope, but I want to prepare a Server and need a ResourceScope to do so.... I don't know why, but when watching various talks on receivers, extension functions and context receivers/parameters, this thought got me hooked quite a lot... since then, I try to be consistent, but it sometimes reads awkwardly when called. Sadly!

[Youssef Shoaib [MOD]]

You'll be happy to know that this PR is now merged, which makes AcquireStep not get in your way anymore!