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

Improved threadsafeness for custom stopword lists. #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

funwithbots
Copy link

If LoadStopWordsFromString() is called concurrently, map writes can collide and panic the application. Refactored to write into a new map and then copy the map into the appropriate language stoplist to reduce concurrency issues.

@funwithbots
Copy link
Author

Not a perfect solution but this should minimize data races. To be completely safe, wrap the call to LoadStopwordsFromFile and LoadStopwordsFromString in a mutex.

@adamdecaf
Copy link

Yea, to be completely safe calls like arabic = stopWordList need to be protected with a mutex.

@funwithbots
Copy link
Author

Yea, to be completely safe calls like arabic = stopWordList need to be protected with a mutex.

Prior to this branch, I could panic my test with 10 go routines. 10k now works but I see the race detector triggered intermittently.

Given that the repo hasn't been updated in years, I didn't want to put a ton of time into refactoring the entire package to allow an instance to be instantiated and reused to make this work 100%. Although, if the package is still used, it's probably worth doing.

@adamdecaf
Copy link

Yep. I totally understand that tradeoff. We use this package in a project called Watchman but do the stopwords processing in an atomic and single goroutine (for other reasons than stopwords) which has avoided this bug.

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.

None yet

2 participants