Skip to content

Conversation

Bogdanp
Copy link
Contributor

@Bogdanp Bogdanp commented Aug 31, 2025

I noticed that yield/no-sync could potentially return void? in addition to #t or #f, which seems like a potential bug.

@mflatt
Copy link
Member

mflatt commented Aug 31, 2025

Good catch, but I think the documentation promises a #t result instead of #f when the current thread is not the handler thread. I'm not sure that was the right choice, but it's more consistent with a (void) result, so probably we want to align with the documentation.

@Bogdanp
Copy link
Contributor Author

Bogdanp commented Aug 31, 2025

Note that this change is about yield/no-sync, which is private and undocumented (afaict). I was concerned about uses like

(lambda () (let loop () (pre-event-sync #t) (when (yield/no-sync) (loop))))
where a void result would cause an infinite loop.

My reading of the documentation for yield is that a #f result is expected in this case (a yield call without a positional arg):

If no argument is provided, yield dispatches an unspecified number of events, but only if the current thread is the current eventspace’s handler thread (otherwise, there is no effect). The result is #t if any events may have been handled, #f otherwise.

It does say #t is expected if called in a non-handler thread, but only if a 'wait positional argument is provided

If v is 'wait [...]
[...]
When called in a non-handler thread, yield returns immediately. In either case, the result is #t.

I could definitely be missing something, though.

@mflatt
Copy link
Member

mflatt commented Sep 1, 2025

Ah, I didn't look at enough context to see that yield/no-sync is a function used only internally. It looks like that function always called via constrained-replay, though, which would make sure that it's called in a handler thread. If I have that right, maybe yield/no-sync should skip the check or complain if it's called in the wrong eventspace.

Meanwhile, I see how the documentation can be read your way — and your way matches the implementation of the non-internal functions. I'll look at revising the docs to make them unambiguous to me.

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.

2 participants