Android - Google's Contradiction on Singleton Pattern

Question

I've been reading a bit about Singleton pattern usage in Android and its disadvantages regarding to keeping the Context. In fact, when I implement the following code:

private static HttpManager sSingleton;
private Context mContext;

private HttpManager(Context context) {

    mContext = context;
}

public static synchronized HttpManager getInstance(Context context) {

    if (sSingleton == null) {
        sSingleton = new HttpManager(context);
    }

    return sSingleton;
}

Android Studio shows me the following warning:

Do not place Android context classes in static fields (static reference to HttpManager which has field mContext pointing to Context); this is a memory leak and also breaks Instant Run.

However, I can see Singletons implemented and recommended in this page of Android's docs.

If your application makes constant use of the network, it's probably most efficient to set up a single instance of RequestQueue that will last the lifetime of your app. You can achieve this in various ways. The recommended approach is to implement a singleton class that encapsulates RequestQueue and other Volley functionality.

Since Google is contradicting itself, can someone guide me and advise me on this point?

Scene

Where's the contradiction? – Brittabrittain

There is no contradiction. The first part says you shouldn't put Android context classes into singletons. Second part says nothing about putting those into singletons. It is important to read the text properly and understand what it says. – Miocene

...that will last the lifetime of your app. probably refers that the object (singleton) will be alive (reference held) as long as the context object is alive, this implies that application context application.getApplicationContext() should be used. – Calchas

@Brittabrittain - If you copy code from Google's docs and paste it into Google's IDE and you get a warning, that's a contradiction. – Delanadelancey

Answer 1

Since Google is contradicting itself

No, it is not.

The quoted Lint warning is not complaining about creating singletons. It is complaining about creating singletons holding a reference to an arbitrary Context, as that could be something like an Activity. Hopefully, by changing mContext = context to mContext = context.getApplicationContext(), you will get rid of that warning (though it is possible that this still breaks Instant Run — I cannot really comment on that).

Creating singletons is fine, so long as you do so very carefully, to avoid memory leaks (e.g., holding an indefinite static reference to an Activity).

Sneakbox

It's weird, because if I create a class with exactly the code you can find in that section (see MySingleton class in the link I mentioned before), Android Studio shows me that warning. So I inferred that Google is contradicting itself when it suggests you to design a class the way they advise not to do... I applied the change you suggest and the warning is still showing; anyway, thank you for your answer :). – Scene

@NadiaCastelli: The Lint check needs refinement. See this issue. – Sneakbox

Great! The incoherence between their documentation and that warning confused me. Thank you very much! – Scene

So if we ignore the warning and supply this.getApplicationContext as context so no memory leak will actually occur, right? – Gemination

@Ozuf: Correct. The Application context is a pre-existing singleton. It is pre-leaked, in effect. You cannot somehow leak it further. :-) – Sneakbox

Thanks man. I actually thought Google was contraditing itself. – Gemination

The lint warning in Android Studio is still there after making your change, this would be due to it not changing the fact of the class still owning a static reference to a context instance. I'm going to simply ignore the warning for now. – Parka

@CommonsWare, I have the same lint warning, but with a ImageButton. I really need it to access a bunch of buttons, one at a time. ImageButton is set to null after usage. Please provide some advice, if you can. – Gleiwitz

@Inoy: "I really need it to access a bunch of buttons, one at a time" -- I really doubt it. Feel free to post a separate question, with a minimal reproducible example, and asking for advice for how to avoid putting an ImageButton in a static field. – Sneakbox

@Sneakbox thanks for fast reply, and here you go - #44169770 – Gleiwitz

"No, it is not." YES IT IS. A warning implies you're doing something wrong. If you copy and paste code from Google's docs into Google's IDE and it gives you a warning, that's a contradiction. – Delanadelancey

Answer 2

This is indeed a contradiction, since in many singletons you will need a context. Have a look at this post, I am using now this approach to avoid the warning in android studio:

Android Singleton with Global Context

Molli