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

KTOR-8194 Fix SelectorManager dispatcher ignored on Native #4671

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Thomas-Vos
Copy link
Contributor

@Thomas-Vos Thomas-Vos commented Feb 13, 2025

KTOR-8194 SelectorManager creates own thread instead of provided dispatcher on Native

Now Native now longer creates a separate thread per SelectorManager, but can use IO pool when IO dispatcher is given, just like JVM.

Also removed Job to make it consistent with JVM.

@Thomas-Vos
Copy link
Contributor Author

Maybe SelectorManager should use IO by default on supported platforms, or document that IO should be passed.

public expect fun SelectorManager(
dispatcher: CoroutineContext = EmptyCoroutineContext
): SelectorManager

@Thomas-Vos Thomas-Vos force-pushed the fix-selectormanager-dispatcher branch 3 times, most recently from 75459e6 to 83ce036 Compare February 13, 2025 13:42
@Thomas-Vos
Copy link
Contributor Author

Thomas-Vos commented Feb 14, 2025

Still have to check why the testEchoByteArray[macosX64] failed. Not sure whether this is caused by the changes in this PR or if the issue was already present. Cannot reproduce locally, currently unsure how this failure is even possible.

https://ktor.teamcity.com/buildConfiguration/Ktor_KtorMatrixNativeMacOSX64/319909

@Thomas-Vos Thomas-Vos marked this pull request as draft February 14, 2025 16:58
@Thomas-Vos Thomas-Vos force-pushed the fix-selectormanager-dispatcher branch from 83ce036 to 90a7cea Compare February 14, 2025 17:07
@Thomas-Vos Thomas-Vos force-pushed the fix-selectormanager-dispatcher branch from 90a7cea to 9546fea Compare February 17, 2025 22:45
@Thomas-Vos
Copy link
Contributor Author

Thomas-Vos commented Feb 17, 2025

Test on CI is no longer failing (two runs), so #4680 fixed it or the test is flaky.

@Thomas-Vos Thomas-Vos marked this pull request as ready for review February 17, 2025 23:22
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

Successfully merging this pull request may close these issues.

1 participant