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

Expose more ThreadPool functionality through Future.withEventualValue(). #1828

Closed

Conversation

player-03
Copy link
Contributor

Previously, Future.withEventualValue() tried to match the Future constructor, with one subtle difference in behavior. It interpreted null to mean "run the function again," which causes an infinite loop if the programmer returns null without realizing. Joshua found a regression of this sort in Lime itself, one I should have caught but overlooked.

This PR changes the function signature to avoid any possibility of this error. There is no return value anymore, so you can't accidentally return the wrong value. Instead, as with ThreadPool, you are given a WorkOutput object and must call sendComplete() at some point. The work function has the same signature as in ThreadPool, so if users understand the one, they'll understand the other.

I have a couple other branches that are closer to how withEventualValue() used to work, and I can bring them back if requested. But withEventualValue() is new in 8.2.0, so it shouldn't hurt to change its signature.

Specifically, the work function you pass to `withEventualValue()` is now the same as the one you'd pass to `ThreadPool.run()`, taking two arguments and returning none. This allows access to `sendProgress()` as well as simplifying the implementation.
Lots of apps won't need them, or will only use one of them, so there's no need to pre-allocate both.
@player-03
Copy link
Contributor Author

Perhaps we should do this through Promise instead.

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