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

fix deallocated clipboard/emoji views #728

Merged
merged 3 commits into from
Apr 21, 2024

Conversation

codokie
Copy link
Contributor

@codokie codokie commented Apr 20, 2024

When keyboard instance is destroyed (such as by switching to another keyboard), postDeallocateMemory() is issued by the handler of LatinIME, and the clipboard and emoji views memory is deallocated after 10 seconds (the delay of the message) even if the keyboard instance is recreated in the meantime.
cancelDeallocateMemory() does not seem to remove the pending message for some reason.
That causes all emojis/clipboard entries to suddenly vanish.

Here is a video taken from #616 showing how to reproduce the bug (credit to @Uranusek)

318530574-e4556051-3029-40b4-a479-ff27fc7df453.mp4

With the small check I altered, the memory of those views will not be deallocated any longer when they are visible

Copy link
Contributor Author

@codokie codokie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public void deallocateMemory() {
    if (mKeyboardView != null) {
        mKeyboardView.cancelAllOngoingEvents();
        mKeyboardView.deallocateMemory();
    }
...

I wonder if the keyboard view should be deallocated even when it is visible?
I have a feeling it shouldn't but I'm not sure if there would be any implications

@Helium314
Copy link
Owner

Helium314 commented Apr 20, 2024

If we go this way, shouldn't mKeyboardView receive a similar check?
[edit: sorry, didn't see your post. So we have the same question]

But I would prefer understanding why the message is not canceled. Maybe this is also happening for other messages, potentially causing other bugs.

@Helium314
Copy link
Owner

Looks like this is a weird interaction of multiple instances of the service:

  1. (LatinIme & handler) instance a is used initially, and destroyed when switching the keyboard. postDeallocateMemory is executed after LatinIME.onDestroy is finished. (apparently the system first destroys the service, and then calls onFinishInputView)
  2. When switching back, instance b is created and initially calls cancelDeallocateMemory, but instance b doesn't have any pending messages.
  3. A few seconds later, instance a receives the message and calls , which somehow finds the way to the current views.

@Helium314
Copy link
Owner

An alternative solution would be to add mHandler.removeAllMessages() at the end of LatinIME's onDestroy

This would be the way to go, the garbage collection should handle the rest anyway.

@Helium314
Copy link
Owner

Still deallocating memory definitely is a good idea, thanks!

@Helium314 Helium314 merged commit 9c2cf04 into Helium314:main Apr 21, 2024
1 check passed
@codokie codokie deleted the deallocated-views branch April 26, 2024 14:45
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