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

Mark lambdas returning values crossinline #425

Open
fzhinkin opened this issue Dec 11, 2024 · 6 comments
Open

Mark lambdas returning values crossinline #425

fzhinkin opened this issue Dec 11, 2024 · 6 comments

Comments

@fzhinkin
Copy link
Collaborator

kotlinx-io has a few functions accepting lambdas that in turn should return some value (mainly, in the unsafe API). It was overlooked initially, and with non-local break & continue coming w/ Kotlin 2.1, it's even more likely to mistakenly exit from the lambda without retuning a value via return/break/continue.

As a safety net, such lambda parameters should be marked crossinline.

@fzhinkin
Copy link
Collaborator Author

With crossinline it won't be possible to call suspend-functions inside the unsafe API call and already Ktor relies on it.

@JakeWharton
Copy link
Contributor

That seems like a good outcome! Suspending in the middle of an unsafe operation means another coroutine scheduled on the same thread could see invalid state.

Larger question is why Ktor depends on an API-unstable library?

@fzhinkin
Copy link
Collaborator Author

Suspending in the middle of an unsafe operation means another coroutine scheduled on the same thread could see invalid state.

[If I got your point right], one can use good old threads (on JVM), initiate some good old blocking IO call, stuck there for a while and some other thread, even though the access to a buffer will be properly synchronized, will observe the invalid state.

It's all about avoiding shared access, and I don't think crossinline can help with that.

Larger question is why Ktor depends on an API-unstable library?

Most likely, for the same reason, for example, Okio uses API-unstable kotlinx-datetime: the library provides required API. ;)

After all, the fact that API is unstable doesn't mean that we're going to break things whenever we want. Experimental status gives us a liability waiver, but we try to be as undisruptive as possible and stay backwards compatible unless the breaking change is unavoidable.

@JakeWharton
Copy link
Contributor

even though the access to a buffer will be properly synchronized

This doesn't sound like proper synchronization, though, if I'm understanding your scenario. Any other thread trying to access the Buffer would get blocked by the unsafe operation being wrapped in synchronized.

Okio uses API-unstable kotlinx-datetime

Well, we do use it, but only for the fake file system which is for testing purposes. The artifacts we expect to be used in published libraries and applications do not.

@fzhinkin
Copy link
Collaborator Author

fzhinkin commented Dec 11, 2024

This doesn't sound like proper synchronization, though, if I'm understanding your scenario. Any other thread trying to access the Buffer would get blocked by the unsafe operation being wrapped in synchronized.

My bad. By "access" I meant "get a reference to the buffer", and by "properly synchronized" I meant "a thread will get the reference without a data race".
Any mechanism enforcing mutually exclusive execution, like Java-synchronized, will indeed help. But crossinline is not one of them.
So I'm not sure if it would be nice to ban suspending calls from unsafe API callbacks only.

@fzhinkin
Copy link
Collaborator Author

The unsafe API could be doubled with suspend versions accepting crossinline suspend lambdas, but that doesn't look nice either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants