Synchronized static method not working in android?

Question

I have one class that needs to be a singleton:

private static StationsFile instance;
private Context ctx;

protected StationsFile(Context ctx){
    this.ctx = ctx;

    load();
}
public static synchronized StationsFile getInstance(Context ctx){
    if(instance == null){
        Log.d("StationsFile", "set instance " + StationsFile.class.hashCode());
        instance = new StationsFile(ctx);
        Log.d("StationsFile", "instance set " + instance.hashCode());
    }
    return instance;
}
protected static StationsFile getInstance(){
    return getInstance(null);
}

private void load(){
    if(externalStorageIsReadable()){
        // Loads from sd file
    }else{
        loadDefaults();
    }
}

private void loadDefaults(){
    if(this.ctx != null){
        // Load from raw file in raws folder (I need a context to access Resources)
    }else{
        // Hardcoded default here
    }
}

Now, as this method is synchronized and it's static, it's supposed to be called only once at a time. When I run this in my device (Android 4.4) there's no problem (in my log i get "set instance - instance set"), but when I run it for the first time (and only the first time after installing) in a Android 2.2 Virtual Device or everytime in a Android 4.4 Virtual Device, I get in my log "set instance - set instance - instance set - instance set", along with a crash because of this.

So it looks like when the start is "slow" it crashes. To me it looks like the synchronized keyword just doesn't work.

This method gets called from 2 different threads at the very start of the app. One thread downloads and parses the file from the internet, so it gets updated, and the other thread (main thread) just reads it. (I don't need one to happen before the other, because if it's not loaded from internet it just reads it from the internal memory).

Is it a bug from the virtual devices? I mean, as it's synchronized (and as far as I understood) this can't be called twice at the same time. How can I fix this?

Edit: refractoring my class and making it follow the singleton pattern magically fixed it. Before I had public static methods that accessed getInstance so "I don't have to put getInstance everywhere I want to call this class". Eventhough, I'm going to rollback my code to find out what was the problem (I'm using a git repo).

Edit2: Using the hashcodes, i get the log: "set instance 1139286928 - set instance 1139286928 - instance set 1139224312 - instance set 1139287568"

Edit3: Found the bug. The problem was that the method loadDefaults when he loaded the file in the Resourcers folder, he parsed that file by using getInstance() again (calling a loadFromString method I had). I thought it were two different threads, but it was the same one, hence why synchronized didn't have any effect on it.

Answer 1

Try this...

// initialized when the class is loaded for the first time
private static final StationsFile instance = new StationFile(); 

public static StationsFile getInstance() {
   return instance; // no need of null check here. No worry about synchronization
}

You might get what the difference is...

Answer 2

So, as I said in the edits, the problem was because I was calling the synchronized method from the same method. It might sound like a recursion, but in this case it wasn't, since there was an optional parameter that made other effects.

If you get the same, just print/log your stack trace in the synchronized method to check wether it traces to another call to the same method.

And don't use static methods to avoid the getInstance() in the rest of the code like I did!

Answer 3

Just make your instance variable final it will fix the problem:

private static final StationsFile instance;

"From Java 6 'Final' variables have special thread-safety semantics, in that other threads are guaranteed to see the final field in at least the state it was in when its constructor finished."