Skip to content

Commit

Permalink
Fix #378: Suppression word may not work
Browse files Browse the repository at this point in the history
This is a corner case missed in the previous CL [1], which aimed to
reload the user dictionary only when necessary.

When the user dictionary reloader thread is not started, suppression
dictionary must be unlocked by main thread (Note: when the reloader
thread is started, the suppression dictionary is unlocked by it).

 [1]: b976132

BUG=#378
TEST=
REF_BUG=26499757,27639932
REF_CL=128960670
REF_TIME=2016-08-01T15:58:29+09:00
REF_TIME_RAW=1470034709 +0900
  • Loading branch information
Noriyuki Takahashi committed Aug 1, 2016
1 parent 73a9e37 commit eee6391
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/data/version/mozc_version_template.bzl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
MAJOR=2
MINOR=18
BUILD=2583
BUILD=2584
REVISION=102
# CAUTION: NACL_DICTIONARY_VERSION is going to be migrated to ENGINE_VERSION.
# NACL_DICTIONARY_VERSION is the target version of the system dictionary to be
Expand Down
21 changes: 13 additions & 8 deletions src/dictionary/user_dictionary.cc
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ class UserDictionary::UserDictionaryReloader : public Thread {
}

// When the user dictionary exists AND the modification time has been updated,
// reloads the dictionary.
void MaybeStartReload() {
// reloads the dictionary. Returns true when reloader thread is started.
bool MaybeStartReload() {
FileTimeStamp modification_time;
if (!FileUtil::GetModificationTime(
Singleton<UserDictionaryFileManager>::get()->GetFileName(),
Expand All @@ -254,13 +254,14 @@ class UserDictionary::UserDictionaryReloader : public Thread {
// Therefore if the file is deleted after first reload,
// second reload does nothing so the content loaded by first reload
// is kept as is.
return;
return false;
}
if (modified_at_ == modification_time) {
return;
return false;
}
modified_at_ = modification_time;
Start("UserDictionaryReloader");
return true;
}

void Run() override {
Expand Down Expand Up @@ -506,11 +507,13 @@ bool UserDictionary::Reload() {
if (reloader_->IsRunning()) {
return false;
}

suppression_dictionary_->Lock();
DCHECK(suppression_dictionary_->IsLocked());
reloader_->MaybeStartReload();

// When the reloader is started, |suppression_dictionary_| is unlocked by the
// reloader. When not started, need to unlock it here.
if (!reloader_->MaybeStartReload()) {
suppression_dictionary_->UnLock();
}
return true;
}

Expand Down Expand Up @@ -598,9 +601,11 @@ bool UserDictionary::Load(
Swap(dummy_empty_tokens);
}

suppression_dictionary_->Lock();
TokensIndex *tokens = new TokensIndex(user_pos_.get(),
suppression_dictionary_);
tokens->Load(storage);
tokens->Load(storage); // |suppression_dictionary_| is unlocked in Load().
DCHECK(!suppression_dictionary_->IsLocked());
Swap(tokens);
return true;
}
Expand Down

0 comments on commit eee6391

Please sign in to comment.