19

Similar question have been asked here, here and here but the context is quite different from this and moreover the code that gave from this error is written by the makers of Android and Android Studio.

This is the code:

public class MySingleton {
    private static MySingleton mInstance;
    private RequestQueue mRequestQueue;
    private ImageLoader mImageLoader;
    private static Context mCtx;

    private MySingleton(Context context) {
        mCtx = context;
        mRequestQueue = getRequestQueue();

        mImageLoader = new ImageLoader(mRequestQueue,
                new ImageLoader.ImageCache() {
            private final LruCache<String, Bitmap>
                    cache = new LruCache<String, Bitmap>(20);

            @Override
            public Bitmap getBitmap(String url) {
                return cache.get(url);
            }

            @Override
            public void putBitmap(String url, Bitmap bitmap) {
                cache.put(url, bitmap);
            }
        });
    }

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

    public RequestQueue getRequestQueue() {
        if (mRequestQueue == null) {
            // getApplicationContext() is key, it keeps you from leaking the
            // Activity or BroadcastReceiver if someone passes one in.
            mRequestQueue = Volley.newRequestQueue(mCtx.getApplicationContext());
        }
        return mRequestQueue;
    }

    public <T> void addToRequestQueue(Request<T> req) {
        getRequestQueue().add(req);
    }

    public ImageLoader getImageLoader() {
        return mImageLoader;
    }
}

The lines giving the warning are:

private static MySingleton mInstance;
private static Context mCtx;

Now if I remove the static keyword, change public static synchronized MySingleton getInstance(Context... to public synchronized MySingleton getInstance(Context... the error disappers but another problem comes up.

I use MySingleton in RecyclerView. So this line

@Override public void onBindViewHolder(final RecyclerView.ViewHolder holder, int position) { ImageLoader imageLoader = MySingleton.getInstance(mContext).getImageLoader();

tells me

Non-static method 'getInstance(android.content.Context)' cannot be refrenced from a static context.

Please anybody knows how to fix this?

Community
  • 1
  • 1
X09
  • 3,827
  • 10
  • 47
  • 92

4 Answers4

29

I found the solution to this in the answer to a similar question answered by CommonsWare

I quote

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

So Google is not actually contracting itself. To fix this, if this.getApplicationContext is supplied as a parameter for the context, then there will be no memory leak.

So in essence, ignore the warning and supply this.getApplicationContext as a parameter for the context.

Community
  • 1
  • 1
X09
  • 3,827
  • 10
  • 47
  • 92
  • 8
    I already have `mContext = context.getApplicationContext()` warning is still there! – Max Nov 14 '16 at 09:49
  • Please can you show the full code? I'll be in a better position to help you if I can see the code. – X09 Nov 15 '16 at 16:56
  • 3
    I can confirm, this does not remove the lint warning. I don't think changing Context to getApplicationContext will work since it is still return an object of Context type. – Pellet Jan 04 '17 at 03:29
  • 1
    @Pellet Yes the lint warning will still be there but since you're using `context.getApplicationContext()` there's no actual memory leak. If your goal is to fix the memory leak, this is the solution... except you have a better alternative. – X09 Jan 04 '17 at 10:27
  • @Max Yes the lint warning will still be there. Just ignore it. – X09 Jan 04 '17 at 10:27
  • 1
    @Ozuf An alternative could be to remove the static method from the class and get an instance of the singleton instead via a dependency injection framework. This could be overkill for your implementation though :) – Pellet Jan 05 '17 at 00:57
  • @X09 What do you think about Application class instead of Context in the constructor? – Sinan Dizdarević Oct 19 '18 at 13:40
  • To suppress warning use @SuppressLint("StaticFieldLeak") – Daniel Aug 13 '20 at 15:42
4

I ended up putting this in AppController which has no warning.

public class AppController extends MultiDexApplication {

    public static Context getContext() {
        return mInstance.getApplicationContext();
    }

    private static AppController mInstance;

    public static synchronized AppController getInstance() {
        return mInstance;
    }

    @Override
    public void onCreate() {
        super.onCreate();

        mInstance = this;

    }
}

So whenever you need it just call AppController.getContext()

Arst
  • 3,098
  • 1
  • 35
  • 42
  • 1
    This leads to the same error in my case. `mInstance` is now the problematic field. And why do you need `getInstance()`? – SapuSeven Jul 07 '17 at 09:11
  • mInstance is null in design time in xml layout with custom views, i know it is scarce somehow, but i lost preview in some xml with custom views :( – Hamid Zandi Jul 21 '17 at 17:53
0

I simply removed the static reference to context and works with no errors:

public class RequestQueueSingleton {

    private RequestQueue requestQueue;
    private final ImageLoader imageLoader;
    private final Context ctx;

    private RequestQueueSingleton(Context context) {
        ctx = context;
        requestQueue = getRequestQueue();

        imageLoader = new ImageLoader(requestQueue,
                new ImageLoader.ImageCache() {
                    private final LruCache<String, Bitmap>
                            cache = new LruCache<>(20);

                    @Override
                    public Bitmap getBitmap(String url) {
                        return cache.get(url);
                    }

                    @Override
                    public void putBitmap(String url, Bitmap bitmap) {
                        cache.put(url, bitmap);
                    }
                });
    }

    public static synchronized RequestQueueSingleton getInstance(Context context) {
        RequestQueueSingleton instance;
        instance = new RequestQueueSingleton(context);
        return instance;
    }

    public RequestQueue getRequestQueue() {
        if (requestQueue == null) {
            // getApplicationContext() is key, it keeps you from leaking the
            // Activity or BroadcastReceiver if someone passes one in.
            requestQueue = Volley.newRequestQueue(ctx.getApplicationContext());
        }
        return requestQueue;
    }

    public <T> void addToRequestQueue(Request<T> req) {
        getRequestQueue().add(req);
    }

    public ImageLoader getImageLoader() {
        return imageLoader;
    }
}
Dmitriy Popov
  • 2,150
  • 3
  • 25
  • 34
Kev506
  • 1
  • 1
-2

This was my solution. There is no need to hold a static reference when you're just returning the instance of the RequestQueue?

public class VolleyRequestQueue {

    private static VolleyRequestQueue mInstance;
    private RequestQueue mRequestQueue;

    private VolleyRequestQueue() {

    }

    public static synchronized VolleyRequestQueue getInstance() {
        if(mInstance == null) {
            mInstance = new VolleyRequestQueue();
        }
        return mInstance;
    }

    public RequestQueue getRequestQueue(Context context) {
        if(mRequestQueue == null) {
            mRequestQueue = Volley.newRequestQueue(context.getApplicationContext());
        }
        return mRequestQueue;
    }
}
Montwell
  • 505
  • 3
  • 12