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: HunspellSpellchecker: multiple initializations #1261

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

miurahr
Copy link
Member

@miurahr miurahr commented Feb 2, 2025

User observed multiple HUNSPELL_CHECKER_INITIALIZED logs when user reloading project. The report is posted in RFE#1769 Spell checker dictionary as a language module.

Pull request type

Please mark github LABEL of the type of change your PR introduces:

  • Bug fix -> [bug]

Which ticket is resolved?

No ticket raised. The issue was posted as comment for the RFE.
https://sourceforge.net/p/omegat/feature-requests/1769/#7093

What does this PR change?

  • Add a regression test
  • Fix a multiple initializations

Other information

@miurahr miurahr added the bug label Feb 2, 2025
Copy link

github-actions bot commented Feb 2, 2025

❌ Quality checks failed.

Please look a Gradle Scan page for details:
https://gradle.com/s/ll7gc3kdvpqau

Copy link

github-actions bot commented Feb 2, 2025

❌ Quality checks failed.

Please look a Gradle Scan page for details:
https://gradle.com/s/xx3j4dfcr37hi

Copy link

github-actions bot commented Feb 2, 2025

❌ Quality checks failed.

Please look a Gradle Scan page for details:
https://gradle.com/s/sgjp2hih4a5ie

@miurahr miurahr changed the title HunspellSpellcheckerTest: multiple initializations fix: HunspellSpellchecker: multiple initializations Feb 27, 2025
@miurahr miurahr requested a review from brandelune February 27, 2025 07:07
@miurahr
Copy link
Member Author

miurahr commented Feb 27, 2025

@brandelune This PR is to fix your report in somewhere about hunspell multiple initialization. I cannot find a log archive or issue ticket.

@brandelune
Copy link
Contributor

@brandelune
Copy link
Contributor

Considering the problem, do you have an idea how to reproduce it ?

@miurahr
Copy link
Member Author

miurahr commented Mar 1, 2025

Considering the problem, do you have an idea how to reproduce it ?

Please look a test case which reproduce the issue.

https://github.com/omegat-org/omegat/pull/1261/files#diff-e3f3f2a34c71532378392bd8dc86083e13e1710a614cff0bd5c3b1d9c938abd4R133-R155

miurahr added 2 commits March 1, 2025 12:07
User observed multiple HUNSPELL_CHECKER_INITIALIZED logs when user reloading project.
The report is posted in RFE#1769 Spell checker dictionary as a language module.
This is a reproducible of the issue.
Fix the case that User observed multiple HUNSPELL_CHECKER_INITIALIZED logs when user reloading project.

Signed-off-by: Hiroshi Miura <[email protected]>
@miurahr miurahr force-pushed the topic/miurahr/spellcheck/fix-event-listener-duplication branch from 23adcb4 to c73b9d9 Compare March 1, 2025 03:07
@miurahr
Copy link
Member Author

miurahr commented Mar 1, 2025

I have rebased on master and recreate commits.

  1. The first commit reproduces the problem.
  2. The 2nd commit fixes the issue.

@miurahr
Copy link
Member Author

miurahr commented Mar 1, 2025

Line-by-line explanation for persons who don't read the java source code.

   HunSpellCheckerMock checker = new HunSpellCheckerMock();

For test purpose, mocking HunSpellShecker class to count how much initialize happened.

   assertThat(checker.initialize()).as("Success initialize").isTrue();

Check succeed the initialization

   assertThat(checker.getInitializeCounter()).as("Hunspell Checker initialized once").isEqualTo(1);

Check the initialization is done only once.

        // Fire project change events twice
        final CountDownLatch latch = new CountDownLatch(2);
        CoreEvents.registerProjectChangeListener(eventType -> latch.countDown());
        CoreEvents.fireProjectChange(IProjectEventListener.PROJECT_CHANGE_TYPE.LOAD);
        CoreEvents.fireProjectChange(IProjectEventListener.PROJECT_CHANGE_TYPE.LOAD);
        latch.await();

Force fire the project load event twice.

   assertThat(checker.getInitializeCounter()).as("Hunspell Checker loaded 3rd times").isEqualTo(3);

Check initialization happened 3 times, at 1st when being initialized above code, 2nd and 3rd with the fired events

Without 2nd commit which fixes the issue, the last check is failed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants