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

Replace ALooper_pollAll with ALooper_pollOnce. #1002

Closed
wants to merge 1 commit into from

Conversation

DanAlbert
Copy link
Member

The pollAll API was removed because it can't be used safely. The recommendation is to replace existing calls to pollAll with pollOnce with an explicit loop.

There are quite a few other samples that are doing this. Fixing one to start so I can check with someone that understands the pollAll bug so I don't accidentally copy the same bug, then I'll fix the others.

#999

The `pollAll` API was removed because it can't be used safely. The
recommendation is to replace existing calls to `pollAll` with `pollOnce`
with an explicit loop.

There are quite a few other samples that are doing this. Fixing one to
start so I can check with someone that understands the `pollAll` bug so
I don't accidentally copy the same bug, then I'll fix the others.

android#999
@DanAlbert
Copy link
Member Author

@jreck says this isn't actually a good fix. The problem is present when calling pollOnce in a loop as I have here as well. The nuance is that this sample doesn't have the bug at all, because the problem only happens when ALooper_wake is used, and this sample doesn't do that. OTOH, this sample doesn't do that mainly because it's trivial. Anything built with this sample as its base likely would.

The better fix is to migrate to AChoreographer. I'll close this and upload a new PR that does that.

@DanAlbert DanAlbert closed this Apr 26, 2024
@DanAlbert DanAlbert deleted the pollAll branch April 26, 2024 20:41
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