6

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?

Adinia
  • 3,722
  • 5
  • 40
  • 58
Nadia Castelli
  • 147
  • 2
  • 11
  • 1
    Where's the contradiction? – Luis Oct 03 '16 at 21:44
  • 2
    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. – Sami Kuhmonen Oct 03 '16 at 21:48
  • `...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. – Nikola Despotoski Oct 03 '16 at 22:33
  • @Luis - If you copy code from Google's docs and paste it into Google's IDE and you get a warning, that's a contradiction. – stepanian Jan 22 '19 at 06:30

2 Answers2

23

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).

CommonsWare
  • 986,068
  • 189
  • 2,389
  • 2,491
  • 1
    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](https://developer.android.com/training/volley/requestqueue.html#singleton)), 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 :). – Nadia Castelli Oct 04 '16 at 02:29
  • 3
    @NadiaCastelli: The Lint check needs refinement. See [this issue](http://code.google.com/p/android/issues/detail?id=223557). – CommonsWare Oct 05 '16 at 17:25
  • Great! The incoherence between their documentation and that warning confused me. Thank you very much! – Nadia Castelli Oct 05 '16 at 18:56
  • So if we ignore the warning and supply `this.getApplicationContext` as context so no memory leak will actually occur, right? – X09 Oct 17 '16 at 19:51
  • 4
    @Ozuf: Correct. The `Application` context is a pre-existing singleton. It is pre-leaked, in effect. You cannot somehow leak it further. :-) – CommonsWare Oct 17 '16 at 20:01
  • Thanks man. I actually thought Google was contraditing itself. – X09 Oct 17 '16 at 20:04
  • 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. – Pellet Jan 04 '17 at 03:47
  • @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. – Inoy May 24 '17 at 20:12
  • @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 [mcve], and asking for advice for how to avoid putting an `ImageButton` in a `static` field. – CommonsWare May 24 '17 at 20:20
  • @CommonsWare **thanks for fast reply**, and here you go - https://stackoverflow.com/questions/44169270/do-not-place-android-context-classes-in-static-fields-this-is-a-memory-leak – Inoy May 24 '17 at 22:09
  • "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. – stepanian Jan 22 '19 at 06:27
-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

Community
  • 1
  • 1
Al Wld
  • 869
  • 7
  • 19