Test depending on core.async thread names will likely break soon

[puredanger]

This test code core-async-dispatch-thread? depends on the name of core.async threads, which is going to change soon and is not considered part of the API, so fyi, it will likely stop working.

[ericdallo]

@puredanger thanks for the heads up, I know the way it's done currently is not ideal, so my question is, is that a better way to know if a thread is the core async dispatch? if not, could you provide the new name so we can make an or while there is not a better way?

[puredanger]

I guess the question is, why do you want to know?

The nature of core.async dispatch threads are going to change as we open up possibilities to leverage Java 21 virtual threads and they will not actually be "special" like this anymore (there won't be a dispatch pool).

[ericdallo]

IIRC it was related with this change, but maybe @ferdinand-beyer can help with more details

[puredanger]

That helps. I think the tests are going a bit overboard in checking things they probably shouldn't be checking by relying on internals of core.async, and there may just be no good way to test some of those things (esp b/c the assumptions here will be incorrect in the future).

[ericdallo]

Yeah makes sense, we will probably need to remove that check or find a better end value to check instead

[ferdinand-beyer]

The issue was that we were running blocking code in the fixed-size dispatcher thread pool. But if that went away, and core.async used virtual threads instead, this limitation would would no longer be a problem. Then we could delete this test, and maybe roll back some of the code.

Please note that there is also a test can-determine-core-async-dispatch-thread which should fail should this internal change. So maybe that is a good indicator when to reconsider this test.

[puredanger]

We are not changing the restrictions around what activity is allowed in a go block - if you are blocking, you still should not be in a go block. We are adding a new io-thread construct (like existing thread) designed for blocking work. Using that would be appropriate.

[ferdinand-beyer]

@puredanger any chance you can consider some way of either detecting whether we're currently in a go block, or maybe a way to provide a custom executor for go blocks that can be used in tests?

I do understand that you don't want to support undocumented features, and ideally people should not make any assumptions on implementation details. However, as the example in lsp4clj shows, it is very easy to get it wrong, and it would be helpful to have some way of ensuring with tests that callbacks are not invoked from within a go block.

EDIT: Maybe it would be easier for us to wait and see how the changes you are describing look like and adapt the code accordingly when they are released. Maybe the answer for lsp4clj would be to not use go blocks in the first place, but something like the io-thread construct you describe.

[puredanger]

One of the new features we're adding in the next release will be the ability to specify the executor services used in core.async, including go blocks.

[ferdinand-beyer]

Sweet!

@ericdallo - I suggest we wait for the new core.async release, and then we could refactor the tests to use a custom executor, allowing us to detect whether we are running on one of its threads. Or we refactor the go-loops in some way that we don't need this anymore.

Happy to support with this refactoring!

[ericdallo]

@ferdinand-beyer sounds great to wait for the release, thanks for the help!