Thread safe singleton

Question

Possible Duplicate:
Thread safety in Singleton

hi,

I am using wordnet dictionary to find the synonyms for words. As I have lots documents I have used multiple threads to do the document pre-processing which involves stemming, stop word removal and synonym replacement.

I am using the following code to access the dictionary and get the wordset for each document.

IndexWordSet set = Dictionary.getInstance().lookupAllIndexWords(newWord);

This works fine in a single threaded environment. But in a multi threaded environment this didn't work as expected. The program gets stuck after some time.

Is this because the Dictionary.getInstance() is a singleton class and it is not thread safe? If so how can I modify the access to the dictionary so that it is thread safe? ( I cannot modify the dictionary class since I have used a dictionary library)

Will making the classes which use the dictionary as singletons help? — Satadru Biswas, Apr 26 '11 at 04:26
you would have to consult he wordnet dictionary api to see if it was thread safe. — MeBigFatGuy, Apr 26 '11 at 04:27
if you determine that this code is not thread safe, you would have to make this call in a synchronized block that all accessors used. `synchronized(someLockingObject) { return Dictionary.getInstance().lookupAllIndexWords(newWord); }` — MeBigFatGuy, Apr 26 '11 at 04:28
the Dictionary itself is a singleton class and it included in a external library. So I cannot change that class. Is there a way to access the singleton dictionary class in a thread safe way when we want to access the dictionary class? — Nuwan, Apr 26 '11 at 04:34
@BrianRoach I think its different than the other question you point out (Thread safety in Singleton). In this case the implementer does not have access to the Dictionary class code he have to deal with a third party component. — Lynch, Apr 26 '11 at 04:39
Did your `!` key get stuck? Don't worry, I'll remove the extra ones for you. — jalf, Apr 26 '11 at 04:48
It's amazing the trouble people go to to make things singletons... Just so users of the class can go through the same amount of trouble just to fix the damage. — jalf, Apr 26 '11 at 04:50
Nuwan, the project is open source, so modifying the code is certainly an option: http://sourceforge.net/projects/jwordnet/ — alphazero, Apr 26 '11 at 04:55

Lynch · Answer 1 · 2011-04-26T04:42:20.860

2

Write a wrapper for your Dictationary instance. In this wrapper synchronize the access to ensure that only one thread can access to lookupAllIndexWords() at a time.

public class DictionaryIndexer {
   public static IndexWordSet lookupAllIndexWords(newWord) {
       final Dictionary instance = Dictionary.getInstance();
       synchronized (instance) {
           return instance.lookupAllIndexWords(newWord);
       }
   }
}

If you encapsulate all the calls to Dictionary with your wrapper using the same lock for synchronization you migth be able to have a thread safe solution.

edited Apr 26 '11 at 04:42

answered Apr 26 '11 at 04:26

Lynch

9,174
2
23
34

1

Be careful with synchronizing in the signature instead of in a block in the method. Other code could synchronize on the class's monitor and lock you out. Note that sometimes this is desireable, but often it isn't. – corsiKa Apr 26 '11 at 04:28
@glowcoder would you suggest to synchronize around Dictionary.getInstance() instead? – Lynch Apr 26 '11 at 04:31
yes, agreed with glowcoder. It is always safer to to use a private field for synchronization if possible, as you know that this class owns the locking object, and some other class can't possibly interfere with your synchronization. – MeBigFatGuy Apr 26 '11 at 04:45
@MeBigFatGuy I can envision some scenarios where you would want to use the default monitor. One would be sealed-package scoped classes, or iteration over an entity. (You wouldn't want something editing the underlying structure while you iterate, but you wouldn't want to copy the structure to iterate over it either.) The latter can be seen with the synchronous collection libraries. Of course, any time you do you need to clearly document the fact that the monitor is exposed. These examples notwithstanding, I would still use a private field until you've identified a need to do otherwise. – corsiKa Apr 26 '11 at 04:56

score 1 · Answer 2 · answered Apr 26 '11 at 05:05

From the source:

You have iterators and state all over this library:

/**
 * Main word lookup procedure. First try a normal lookup. If that doesn't work,
 * try looking up the stemmed form of the lemma.
 * @param pos the part-of-speech of the word to look up
 * @param lemma the lemma to look up
 * @return IndexWord the IndexWord found by the lookup procedure, or null
 *              if an IndexWord is not found
 */
public IndexWord lookupIndexWord(POS pos, String lemma) throws JWNLException {
    lemma = prepareQueryString(lemma);
    IndexWord word = getIndexWord(pos, lemma);
    if (word == null && getMorphologicalProcessor() != null) {
        word = getMorphologicalProcessor().lookupBaseForm(pos, lemma);
    }
    return word;
}



/**
 * Return a set of <code>IndexWord</code>s, with each element in the set
 * corresponding to a part-of-speech of <var>word</var>.
 * @param lemma the word for which to lookup senses
 * @return An array of IndexWords, each of which is a sense of <var>word</var>
 */
public IndexWordSet lookupAllIndexWords(String lemma) throws JWNLException {
    lemma = prepareQueryString(lemma);
    IndexWordSet set = new IndexWordSet(lemma);
    for (Iterator itr = POS.getAllPOS().iterator(); itr.hasNext();) {
        IndexWord current = lookupIndexWord((POS)itr.next(), lemma);
        if (current != null) set.add(current);
    }
    return set;
}

and in POS we find

private static final List ALL_POS =
    Collections.unmodifiableList(  /* alphazero: this is good news .. */
            Arrays.asList(new POS[] {NOUN, VERB, ADJECTIVE, ADVERB}));

public static List getAllPOS() {
    return ALL_POS;
}

Try Lynch's answer. It should work.

score 0 · Answer 3 · answered Apr 26 '11 at 04:25

0

You can use one of the concurrent containers...

Alternately you can use synchronization within the singleton instance (somebody already posted a link for the thread safe singleton in the comments).

answered Apr 26 '11 at 04:25

Kiril

39,672
31
167
226

it's not his container, so he can't change the implementation to user concurrent containers. Synchronization is the answer. – MeBigFatGuy Apr 26 '11 at 04:31
@MeBigFatGUy If the look-up functionality is too complex, then you may be right... but if the functionality is a simple look-up in a hash table, then changing to a concurrent or lock-free container wold work better than synchronizing the singleton instance. – Kiril Apr 26 '11 at 12:21

Thread safe singleton

3 Answers3