Warning: Do not place Android context classes in static fields; this is a memory leak (and also breaks Instant Run)

Question

Android Studio:

Do not place Android context classes in static fields; this is a memory leak (and also breaks Instant Run)

So 2 questions:

#1 How do you call a startService from a static method without a static variable for context?
#2 How do you send a localBroadcast from a static method (same)?

Examples:

public static void log(int iLogLevel, String sRequest, String sData) {
    if(iLogLevel > 0) {

        Intent intent = new Intent(mContext, LogService.class);
        intent.putExtra("UPDATE_MAIN_ACTIVITY_VIEW", "UPDATE_MAIN_ACTIVITY_VIEW");
        mContext.startService(intent);
    }
}

or

        Intent intent = new Intent(MAIN_ACTIVITY_RECEIVER_INTENT);
        intent.putExtra(MAIN_ACTIVITY_REQUEST_FOR_UPDATE, sRequest));
        intent.putExtra(MAIN_ACTIVITY_DATA_FOR_VIEW, sData);
        intent.putExtra(MAIN_ACTIVITY_LOG_LEVEL, iLogLevel);
        LocalBroadcastManager.getInstance(mContext).sendBroadcast(intent);

What would be the correct way to do this without using mContext?

NOTE: I think my main question might be how to pass context to a class from which the calling method lives.

Limassol

Can't you pass Context as a parameter in the method? – Plenty

I would be calling this routine in places that wouldn't have context as well. – Limassol

#1 pass it as a parameter #2 the same. – Petty

Then you have to pass the context to the caller method too. The problem is that static fields are not garbage collected, so you could leak an activity with all of its Views – Plenty

What I might be missing is how to pass context to a class. – Limassol

@JohnSmith Cascade it from the initiating activity (via constructor parameters or method parameters) right up to the point you need it. – Sphalerite

When you using jni with c and call SharedPreferences in c, it looks impossible to not place Android context classes in static fields.I even use static java method to get a Android context classes. github.com/golang/mobile/blob/master/bind/java/LoadJNI.java . – Wiener

According to this comment which links to this issue report, that warning could be considered spurious sometimes. – Expressive

Answer 1

Simply pass it as a parameter to your method. There is no sense in creating a static instance of Context solely for the purpose of starting an Intent.

This is how your method should look:

public static void log(int iLogLevel, String sRequest, String sData, Context ctx) {
    if(iLogLevel > 0) {

        Intent intent = new Intent(ctx, LogService.class);
        intent1.putExtra("UPDATE_MAIN_ACTIVITY_VIEW", "UPDATE_MAIN_ACTIVITY_VIEW");
        ctx.startService(intent);
    }
}

Update from comments on question: Cascade the context from the initiating activity (via constructor parameters or method parameters) right up to the point you need it.

Sphalerite

Can you provide a constructor example? – Limassol

if your class name is MyClass add a public constructor just like a method to it public MyClass(Context ctx) { // put this ctx somewhere to use later } (This is your constructor) Now create new instance of MyClass using this constructor e.g MyClass mc = new MyClass(ctx); – Sphalerite

I don't think it's as simple to pass-on-demand. Though there are apparent benefits like not having to worry about a stale context or like here, a static one. Let's say you need context [may be you want to write to prefs] in a response callback which will be invoked asynchronously. So at times, you are forced to put it in a member field. And now you have to think how to not make it static. https://mcmap.net/q/202806/-warning-do-not-place-android-context-classes-in-static-fields-this-is-a-memory-leak-and-also-breaks-instant-run seems to work. – Nonobedience

Is it OK to use ApplicationContext as a static field? Unlike activities, application object doesn't get destroyed, right? – Misjudge

but, the question is, why does it memory leak at all to begin with? – Badtempered

Bad idea. If you pass the context to you method as a parameter, where to get the context if i want to call the method? In the end the context will be stored everywhere, and every class need to create/delete after activity is deleted, a lot of more problems to solve than just store it in one global variable (current activity), and set it in onResume or onCreate. – Wiener

Answer 2

Just make sure you pass context.getApplicationContext() or call getApplicationContext() on any context that is passed in via methods/constructor to your singleton if you decide to store it in any member field.

idiot proof example (even if someone would pass in an activity it will grab the app context and use that to instantiate the singleton):

public static synchronized RestClient getInstance(Context context) {
    if (mInstance == null) {
        mInstance = new RestClient(context.getApplicationContext());
    }
    return mInstance;
}

getApplicationContext() according to the docs: "Return the context of the single, global Application object of the current process."

It means that the context returned by "getApplicationContext()" will live through the entire process and thus it does not matter if you store a static reference to it anywhere since it will always be there during the runtime of your app (and outlive any objects/singletons instantiated by it).

Compare that to the context inside of views/activities holding large amounts of data, if you leak a context held by an activity, the system won't be able to free that resource which obviously is not good.

A reference to an activity by its context should live the same life cycle as the activity itself, otherwise it will hold the context hostage causing a memory leak (which is the reason behind the lint warning).

EDIT: To the guy bashing the example from the docs above, there's even a comment section in the code about what I just wrote about:

    // getApplicationContext() is key, it keeps you from leaking the
    // Activity or BroadcastReceiver if someone passes one in.

Filibertofilibuster

to the guy bashing the guy who bashed the example above: the point of this thread is the Lint warning in conflict with Google's own recommended pattern of creating singleton. – Parke

Read: "Do not place Android context classes in static fields; this is a memory leak (and also breaks Instant Run)" Do you know what context classes are? Activity is one of them, and you should not store Activity as a static field, as you described yourself (or it will leak memory). However, you can store Context (as long as it is the application context) as a static field, since it outlives everything. (And thus ignore the warning). I'm sure we can agree upon this simple fact, right? – Filibertofilibuster

as an iOS vet, in my first week of Android... Explanations like this help me understand this context nonsense.. So, that lint warning (oh, how I dislike any warnings) will hang around, but your answer solves the real problem. – Syllabogram

@Marcus if your child class is not aware of who's instantiating it with which context, then it's just bad practice to store it as a static member. furthermore, the application context lives as part of the Application object of your app, the application object will not stay in memory forever, it will get killed. contrary to popular belief, the app won’t be restarted from scratch. Android will create a new Application object and start the activity where the user was before to give the illusion that the application was never killed in the first place. – Parke

@RaphaelC do you have documentation of such? That seems that is completely wrong because Android assures only one Application context per run of each process. – Warsle

@Warsle The Lint message says that placing any Android context in static fields causes memory leaks (see the original subject of this thread). it doesn't say anything about only one application context per "process". btw, an application "process" in Android might get killed at any time by the OS in order to claim memory. an android process doesn't necessarily assure the application context instance will always be the same since the OS may kill an application process at any moment in order to claim memory. – Parke

I dont understand the controversy but, this really stopped the problem, great answer – Badtempered

@RaphaelC it is fine if you use the application context, just look at Android's LocalBroadcastManager code, which does exactly that. And if you call getApplicationContext in the constructor rather that in the getInstance, the warning is gone. – Cattan

Answer 3

Use WeakReference to store the Context in Singleton classes & the warning will be gone

private WeakReference<Context> context;

//Private contructor
private WidgetManager(Context context) {
    this.context = new WeakReference<>(context);
}

//Singleton
public static WidgetManager getInstance(Context context) {
    if (null == widgetManager) {
        widgetManager = new WidgetManager(context);
    }
    return widgetManager;
}

Now you can access Context like

  if (context.get() instanceof MainActivity) {
            ((MainActivity) context.get()).startActivityForResult(pickIntent, CODE_REQUEST_PICK_APPWIDGET);
        }

Intake

this works but better to refer to MainActivity in the WeakReference rather than a Context? private static WeakReference<MainActivity> activityReference. Then simply MainActivity main = activityReference.get(); – Ruckman

Answer 4

It's just a warning. Don't worry. If you wanna use an application context, you can save it in a "singleton" class, which is used to save all of singleton class in your project.

Merna

Answer 5

Generally, avoid having context fields defined as static. The warning itself explains why: It's a memory leak. Breaking instant run may not be the biggest problem on the planet though.

Now, there are two scenarios where you'd get this warning. For an instance (the most obvious one):

public static Context ctx;

And then there's the bit more tricky one, where the context is wrapped in a class:

public class Example{
    public Context ctx;
    //Constructor omitted for brievety 
}

And that class is defined as static somewhere:

public static Example example;

And you'll get the warning.

The solution itself is fairly simple: Do not place context fields in static instances, whether that of a wrapping class or declaring it static directly.

And the solution to the warning is simple: don't place the field statically. In your case, pass the context as an instance to the method. For classes where multiple Context calls are made, use a constructor to pass the context (or an Activity for that matter) to the class.

_{Note that it is a warning, not an error. Should you for whatever reason need a static context, you can do it. Though you create a memory leak when you do so.}

Microscopic

how can we do it without creating a memory leak? – Siccative

You can't. If you absolutely need to pass contexts around, you could look into an event bus – Microscopic

ok this was the problem I was having if you could please take a look at it maybe theres another way of doing it, btw the method has to be static because I am calling it from c++ code #54684363 – Siccative

Answer 6

In your case it does not make much sense to have it as static field but I don't think it's bad in all cases. If you now what are you doing you can have static field that has context and null it later. I am creating static instance for my main model class that has context inside, its application context not activity context and also i have static instance field of class containing Activity that I null in on destroy. I don't see that I have memory leak. So if some clever guy think i am wrong feel free to comment...

Also Instant Run works here fine...

Bradberry

I don't think you're wrong on the principle, but you need to be extra careful that the Activity you're talking about only has a maximum of one single instance at any given time before it can use static fields. If your app ends up with more than one back stack because it can be launched from different places (notification, deep linking, ...), things will go wrong unless you use some flag like singleInstance in the manifest. So it's always easier to avoid static fields from Activities. – Forgery

android:launchMode="singleTask" should be sufficient, so I am switching to that, I used singleTop but did no know its no enough because I want always just single instances of my main activities, thats how my apps are designed. – Bradberry

"singleTask" only guarantees one instance per task. If your app has multiple entry points like deep linking, or launching it from a notification, you may end up with multiple tasks. – Forgery

Answer 7

If you make sure it's an Application Context. It doest matter. Add this

@SuppressLint("StaticFieldLeak")

Ankle

I would not recommend to do this anyway. If you need context you can use requireContext() method, if you use AndroidX libs. Or you can pass the Context directly to the method that needs it. Or you can even just get app's class reference, but I'd rather recommend not to use such SuppressLint suggestion. – Sinister

Answer 8

You can set App instance as static property. For example:

public class App extends MultiDexApplication {
    private static App context;

    public void onCreate() {
        super.onCreate();
        App.context = this;
    }

    public static Context getAppContext() {
        return App.context;
    }
}

Warning will be gone. But I don't know that is this best way.

Brisco