2

I wrote a book reader app that uses a view pager with FragmentStatePagerAdapter. There is no obvious runtime bug with it. However, the memory use keeps increasing when the pages are swiped. So I tested out a simple version of the setup and test it out on a Nexus 7. The user is to alternate between a few swipes and a few rotations. After doing these multiple times, it is noticed from the log that the heap use keeps increasing. I would think it is suggestive of a memory leak. However, I cannot spot the cause of it. Attached are the code, the log of GC, and two leak suspects from MAT. Any helpful insight will be appreciated. Thanks.

MainActivity.java:

package com.example.leakypager;
import android.os.Bundle;
import android.support.v4.app.Fragment;
import android.support.v4.app.FragmentActivity;
import android.support.v4.app.FragmentManager;
import android.support.v4.app.FragmentStatePagerAdapter;
import android.support.v4.view.ViewPager;
import android.util.Log;

public class MainActivity extends FragmentActivity {

    public static final String TAG = "MainActivity";
    public static final String EXTRA_argument = "pageNumber";
    private static final int viewPagerId = 1;
    private static final int totalPages = 10;

    private int mPageNumber;
    private ViewPager mViewPager; 
    private int mTotalPages;

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

        // Set a default value to page 1
        mPageNumber = getIntent().getIntExtra(EXTRA_argument, 1);

        Log.i(TAG, "Called onCreate() on page number " + mPageNumber);

        mViewPager = new ViewPager(this);
        mViewPager.setId(viewPagerId);
        setContentView(mViewPager);

        mTotalPages = totalPages;

        FragmentManager fragmentManager;
        fragmentManager = getSupportFragmentManager();      

        mViewPager.setAdapter(new FragmentStatePagerAdapter(fragmentManager) {
            @Override
            public Fragment getItem(int pos) {
                // The page number is one greater than the pager index.
                Fragment pageFragment;
                pageFragment = PageFragment.newInstance(pos+1);                 
                return pageFragment;
            }

            @Override
            public int getCount() {
                return mTotalPages;  
            }

            @Override
            public int getItemPosition(Object item) {
                return POSITION_NONE;
            }
        });

        if (mPageNumber >= 1 && mPageNumber <= mTotalPages) {
            // The pager item index is one less than the page number.
            mViewPager.setCurrentItem(mPageNumber-1); 
        }
    }
}

PageFragment.java:

package com.example.leakypager;

import android.os.Bundle;
import android.support.v4.app.Fragment;
import android.text.Editable;
import android.text.TextWatcher;
import android.util.Log;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.view.WindowManager;
import android.widget.EditText;
import android.widget.TextView;


public class PageFragment extends Fragment {
    private static final String EXTRA_int = "argument";
    private static final String TAG = "PageFragment";

    private int mPageNumber;
    private String mData;
    private String mNote = "";

    private TextView mDataView;
    private EditText mNoteView;


    public static PageFragment newInstance(int pagenumber) {
        Bundle args = new Bundle();
        args.putInt(EXTRA_int, pagenumber);
        PageFragment fragment = new PageFragment();
        fragment.setArguments(args);
        return fragment;
    }

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);     
        mPageNumber = (Integer) getArguments().getInt(EXTRA_int);
        mData = "This is page " + mPageNumber;
    }

    public View onCreateView(LayoutInflater inflater, ViewGroup parent, Bundle savedInstanceState) {
        Log.i(TAG, "onCreateView() on page " + mPageNumber);
        View v;

        v = inflater.inflate(R.layout.fragment_page, parent, false);

        mDataView = (TextView) v.findViewById(R.id.fragmentPage_data_id);
        mNoteView = (EditText) v.findViewById(R.id.fragmentPage_note_id);

        mNoteView.addTextChangedListener(new TextWatcher() {
            @Override
            public void afterTextChanged(Editable arg0) {}
            @Override
            public void beforeTextChanged(CharSequence arg0, int arg1, int arg2, int arg3) {}

            @Override
            public void onTextChanged(CharSequence c, int start, int before, int count) {
                if (c==null) c="";  // do not allow c to be null
                mNote = c.toString().trim();
            }
        });
     getActivity().getWindow().setSoftInputMode( WindowManager.LayoutParams.SOFT_INPUT_STATE_ALWAYS_HIDDEN);
        mDataView.setText(mData);
        mNoteView.setText(mNote);       

        return v;
    } // end to implementing onCreateView() for PageFragment

}

The layout file contains just one textview and one edit text inside a LinearLayout. (Not reproduced here unless requested).

The log with GC filter shows the following result:

12-06 00:07:03.205: D/dalvikvm(23276): GC_FOR_ALLOC freed 232K, 4% free 7758K/8052K, paused 16ms, total 18ms
12-06 00:07:10.095: D/dalvikvm(23276): GC_FOR_ALLOC freed 272K, 5% free 7989K/8324K, paused 19ms, total 19ms
12-06 00:07:20.945: D/dalvikvm(23276): GC_FOR_ALLOC freed 460K, 7% free 8043K/8564K, paused 21ms, total 22ms
12-06 00:07:23.805: D/dalvikvm(23276): GC_FOR_ALLOC freed 511K, 7% free 8043K/8616K, paused 17ms, total 17ms
12-06 00:07:42.535: D/dalvikvm(23276): GC_FOR_ALLOC freed 421K, 6% free 8134K/8616K, paused 18ms, total 21ms
12-06 00:08:03.925: D/dalvikvm(23276): GC_FOR_ALLOC freed 537K, 7% free 8109K/8708K, paused 20ms, total 20ms
12-06 00:08:11.665: D/dalvikvm(23276): GC_FOR_ALLOC freed 537K, 8% free 8083K/8708K, paused 27ms, total 30ms
12-06 00:08:15.265: D/dalvikvm(23276): GC_FOR_ALLOC freed 456K, 7% free 8138K/8708K, paused 18ms, total 18ms
12-06 00:08:20.015: D/dalvikvm(23276): GC_FOR_ALLOC freed 313K, 5% free 8315K/8708K, paused 25ms, total 26ms
12-06 00:08:24.685: D/dalvikvm(23276): GC_FOR_ALLOC freed 448K, 6% free 8378K/8888K, paused 18ms, total 19ms
12-06 00:08:27.955: D/dalvikvm(23276): GC_FOR_ALLOC freed 330K, 5% free 8560K/8952K, paused 28ms, total 28ms
12-06 00:08:36.055: D/dalvikvm(23276): GC_FOR_ALLOC freed 449K, 6% free 8625K/9136K, paused 37ms, total 37ms
12-06 00:08:42.015: D/dalvikvm(23276): GC_FOR_ALLOC freed 438K, 6% free 8699K/9200K, paused 60ms, total 63ms
12-06 00:08:45.775: D/dalvikvm(23276): GC_FOR_ALLOC freed 454K, 6% free 8760K/9276K, paused 27ms, total 30ms
12-06 00:08:51.185: D/dalvikvm(23276): GC_FOR_ALLOC freed 491K, 6% free 8771K/9328K, paused 36ms, total 36ms
12-06 00:08:57.845: D/dalvikvm(23276): GC_FOR_ALLOC freed 382K, 5% free 8866K/9328K, paused 28ms, total 30ms
12-06 00:09:03.885: D/dalvikvm(23276): GC_FOR_ALLOC freed 491K, 6% free 8887K/9440K, paused 26ms, total 27ms
12-06 00:09:06.905: D/dalvikvm(23276): GC_FOR_ALLOC freed 442K, 6% free 8955K/9460K, paused 26ms, total 27ms
12-06 00:09:11.215: D/dalvikvm(23276): GC_FOR_ALLOC freed 408K, 5% free 9058K/9528K, paused 23ms, total 25ms
12-06 00:09:19.355: D/dalvikvm(23276): GC_FOR_ALLOC freed 445K, 6% free 9130K/9640K, paused 47ms, total 47ms
12-06 00:09:24.965: D/dalvikvm(23276): GC_FOR_ALLOC freed 479K, 6% free 9208K/9748K, paused 29ms, total 30ms
12-06 00:09:30.735: D/dalvikvm(23276): GC_FOR_ALLOC freed 456K, 6% free 9330K/9848K, paused 35ms, total 36ms
12-06 00:09:39.765: D/dalvikvm(23276): GC_FOR_ALLOC freed 473K, 6% free 9477K/10012K, paused 37ms, total 37ms
12-06 00:09:54.795: D/dalvikvm(23276): GC_FOR_ALLOC freed 344K, 4% free 9803K/10208K, paused 36ms, total 36ms
12-06 00:10:05.605: D/dalvikvm(23276): GC_FOR_ALLOC freed 478K, 6% free 10104K/10644K, paused 28ms, total 28ms
12-06 00:10:17.165: D/dalvikvm(23276): GC_FOR_ALLOC freed 499K, 6% free 10483K/11044K, paused 40ms, total 41ms
12-06 00:10:28.855: D/dalvikvm(23276): GC_FOR_ALLOC freed 679K, 7% free 10811K/11552K, paused 44ms, total 46ms
12-06 00:10:43.815: D/dalvikvm(23276): GC_FOR_ALLOC freed 873K, 8% free 11054K/11988K, paused 47ms, total 47ms
12-06 00:11:02.155: D/dalvikvm(23276): GC_FOR_ALLOC freed 891K, 8% free 11360K/12312K, paused 41ms, total 41ms
12-06 00:11:22.725: D/dalvikvm(23276): GC_FOR_ALLOC freed 807K, 7% free 11852K/12720K, paused 58ms, total 58ms
12-06 00:11:46.165: D/dalvikvm(23276): GC_FOR_ALLOC freed 932K, 8% free 12382K/13376K, paused 45ms, total 45ms

Here are two interesting leak suspects reported by MAT:

Suspect 1. 
49 instances of "android.support.v4.app.Fragment$SavedState", 
loaded by "dalvik.system.PathClassLoader @ 0x420ebce8" occupy 
1,892,544 (16.76%) bytes. These instances are referenced from 
one instance of "java.lang.Object[]", loaded by "<system class loader>"

Keywords
java.lang.Object[]
android.support.v4.app.Fragment$SavedState
dalvik.system.PathClassLoader @ 0x420ebce8


Suspect 2.
19 instances of "com.example.leakypager.MainActivity", loaded by
"dalvik.system.PathClassLoader @ 0x420ebce8" occupy 1,419,320 (12.57%) bytes.
Biggest instances:
com.example.leakypager.MainActivity @ 0x422adfa0 - 246,896 (2.19%) bytes.
com.example.leakypager.MainActivity @ 0x4238a430 - 237,880 (2.11%) bytes.
com.example.leakypager.MainActivity @ 0x420edc60 - 160,600 (1.42%) bytes.

Keywords
dalvik.system.PathClassLoader @ 0x420ebce8
com.example.leakypager.MainActivity

(The following update is added on Dec 17, 2013) After some more investigations with much help of Martin (Thanks!), I found out that there are two culprits. The first is the TextWatcher (TextChangedListener) that has not been released. That's an easy fix. The second culprit is very nasty.

The second culprit comes from the EditText that still keeps a reference (mContext) to the activity that has been destroyed. This problem occurs only when the app is tested on Nexus 7 running Android 4.3. It does not occur in Samsung TF running 4.1.3. MAT shows the following incoming links from the existing activity to the dead activity:

com.example.leakySimplePager.MainActivity @ 0x4213b220 (dead)
'- mContext android.widget.EditText @ 0x42114ec8                
 '- mTextView android.widget.Editor @ 0x4214b4b0
  '- this$0 android.widget.Editor$EasyEditSpanController @ 0x420fafa0
   '- [2] java.lang.Object[13] @ 0x420e4a08  
    '- mSpans android.text.SpannableString @ 0x42129b90    
     '- text android.widget.TextView$SavedState @ 0x42129b68      
      '- [2] java.lang.Object[13] @ 0x42129b20        
       '- mValues android.util.SparseArray @ 0x42129ab8          
        '- value java.util.HashMap$HashMapEntry @ 0x4210e420            
         '- [2] java.util.HashMap$HashMapEntry[4] @ 0x4210e3f8              
          '- table java.util.HashMap @ 0x4210e3c0                
           '- mMap android.os.Bundle @ 0x4210e398                  
            '- mState android.support.v4.app.Fragment$SavedState @ 0x42129bb0                    
             '- [0] java.lang.Object[12] @ 0x42123300                      
              '- array java.util.ArrayList @ 0x42106830                        
               '- mSavedState com.example.leakySimplePager.MainActivity$ViewPagerAdapter @ 0x42114e90
                '- mViewPagerAdapter com.example.leakySimplePager.MainActivity @ 0x42140910 (alive)   
                 '- mContext com.android.internal.policy.impl.PhoneWindow$DecorView @ 0x420efc78                              
                  '- mCurRootView android.view.inputmethod.InputMethodManager @ 0x42102ee8                                

The connection between the live activity is through the SavedState of every TextView (EditText is a TextView), that is saved automatically by the system when the view has changes. (Reference: http://developer.android.com/guide/components/activities.html#SavingActivityState) This unfortunate SavedState is being referenced by the FragmentStatePagerAdapter as part of its job. Unlike the listener, I cannot release the FragmentStatePagerAdapter from the ViewPager when the activity is destroyed.

I've observed that this problem not only occurs with EditText, but also with a TextView that supports change to its text formats/color etc. Somehow, since the TextView cannot release its mContext, my current get-around is to explicitly stop it from saving its state by calling mNoteView.setSaveEnabled(false); when the view is created. It does not hurt my actual app since I already save, load/reload the data of the EditText whenever the fragment is back to active. However, Android developers' documentation does not seem to encourage this kind of get-around. Perhaps there is a better way to do it. Any new insights or expert advice will be welcomed!

Ahui
  • 43
  • 1
  • 6
  • Why do you do… @Override public int getItemPosition(Object item) { return POSITION_NONE; } that is very inefficient. – Martin Marconcini Dec 06 '13 at 06:10
  • In the original app, I need to refresh the positions each time because there are additional features such as a figure, a screen layout etc, on all the pages that could change when the user chooses certain options from a menu. Those features are not included in this simple code because I want to focus on testing the memory use of the viewpager. – Ahui Dec 06 '13 at 15:51
  • Understood. I still haven't really seen anything strange here… have you, for the sake of testing, tried temporarily switching to a FragmentPagerAdapter instead of the State version? – Martin Marconcini Dec 06 '13 at 22:03
  • I didn't think of that! Will try it. But I did turn off the @Override getItemPosition(Object item) { return POSITION_NONE; } and tested again. Same phenomenon. I am suspecting that some of the activities are not GCed because some SavedStates are not disposed of. However, when I track the value of the mSavedState array inside FragmentStatePagerAdapter, it never seems to get bigger than 10 (which is expected since there are only 10 pages). – Ahui Dec 06 '13 at 22:43

1 Answers1

0

I'm not sure what's going on, but I'm going to start throwing Ideas in an Answer in the hopes that you can find the problem (a lot easier than comments).

1) Try switching to a FragmentPagerAdapter (which works differently). I don't think that will change much.

2) Improve your Fragment by making more final stuff… e.g.:

final View v = inflater.inflate(R.layout.fragment_page, parent, false);

both mDataView and mNoteView are fields, can they be locals?

Try making:

final TextView dataView = (TextView) v.findViewById(R.id.fragmentPage_data_id);
final EditText noteView = (EditText) v.findViewById(R.id.fragmentPage_note_id);

The final will make them available in your anonymous class.

I'm a little bit suspicious about this (inside each fragment and inside the onCreateView:

getActivity().getWindow().setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_STATE_ALWAYS_HIDDEN);

I'd rather have a method in my Activity that the Fragments can call and let the Activity deal with that (also it would be one line of code in one logical place, the Activity, which can control the Window.

And also if you only change the DataView and EditView in here, and you are able to make them final, then fix these lines too:

    dataView.setText(mData);
    noteView.setText(mNote);       

Finally, the thing I'd personally change is in your activity…

I'd totally create an Inner class for that Fragment Adapter instead of using an Anonymous.

Create a field to hold it:

ViewPagerAdapter mViewPagerAdapter;

(this would be your inner class)

private class ViewPagerAdapter extends FragmentStatePagerAdapter {

and then instantiate it later in your onCreate:

mPagerAdapter = new ViewPagerAdapter(getSupportFragmentManager());
mViewPager.setAdapter(mPagerAdapter);

Also, drop the instance of the FragmentManager because you don't seem to use it again, save the memory ;)

This will also let you do stuff like: mPagerAdapter.notifyDataSetChanged(); in case you need to.

So those are things I would have done this way, try and let me know if you see any change .

Martin Marconcini
  • 26,875
  • 19
  • 106
  • 144
  • Thanks. I was able to incorporate some of the suggestions, but not all because the original app requires certain things to be done that way. After examining the paths to GC root in MAT, it is found that a SavedState has a pointer to a bundle which (after many many links) lead to an EditText, which points to an Activity. After I removed the EditText, the memory problem disappears. Honestly, it does not make sense to me yet. But will keep exploring until a more satisfactory answer is available. – Ahui Dec 07 '13 at 03:40
  • I might have been misled when I thought the leak is caused by an EditText. Still haven't found an explanation yet. But will continue working on it. – Ahui Dec 07 '13 at 04:52
  • Thanks a lot, Martin! After more investigations, I think the problem is really related to the EditText. Once I replace it, there is no more leak. The following discussion seems to be closely related to my problem. Even the paths to GC root shown in MAT are very similar: http://stackoverflow.com/questions/14069501/edittext-causing-memory-leak?lq=1 – Ahui Dec 07 '13 at 21:09
  • Glad to know you have found the culprit ! :) – Martin Marconcini Dec 09 '13 at 18:52