5

I have issue with passing data to fragments. It crashes 0.1% of all times on production. Let's say on 100k opening of activity it happens 100 times. It looks like not very often, but it very bothering me and I think that I am doing something wrong with fragments initialization with data. The thing is, that I create fragments only one time, and all other times I need to pass data to them I am doing it next way: myFragmentInstance.setData(Object someData); And crash happens because it tells that those view elements in fragment are not found and they are NULL, but everything should be fine if I have not recreated them. I am not rotating my phone, or have not enough of memory on it. It happens on network reconnect, because on network reconnect I am going to server for fresh data and then set that new data to my fragments. I have photos of fields of two fragments I use, maby some of you know what that data can tell about status of fragments at the moment of crash. I am using library ButterKnife to initialize fields of fragments and activities, not initializing it with findById, maby it has some influence or no?

Here is link to simple project (only this issue on github): https://github.com/yozhik/Reviews/tree/master/app/src/main/java/com/ylet/sr/review

Description:

CollapsingActivity - activity with Collapsing AppBarLayout. Which loads one or two fragments into "fragment_content_holder" and it has TabLayout to switch between fragments in view pager.

In activity method onCreate() - I'm just simulating request to server (loadData), and when some fake data is loaded - I am showing fragments in view pager, on first call - I am creating new TabMenuAdapter extends FragmentPagerAdapter, populate it with fragments and save links to instances. On the next call - I don't create fragments from scratch and just populate them with fresh data.

MenuFragment1, MenuFragment1 - two fragments. MenuFragment1 - has method public void setupData(SomeCustomData data), to set new data, not recreating fragment on network reconnect.

NetworkStateReceiver - listens to network change and send notifications.

TabMenuAdapter - just simple class to hold fragments.

05-11 18:11:05.088 12279-12279/com.myProjectName E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.myProjectName, PID: 12279
    java.lang.IllegalStateException: Fatal Exception thrown on Scheduler.
        at io.reactivex.android.schedulers.HandlerScheduler$ScheduledRunnable.run(HandlerScheduler.java:111)
        at android.os.Handler.handleCallback(Handler.java:739)
        at android.os.Handler.dispatchMessage(Handler.java:95)
        at android.os.Looper.loop(Looper.java:135)
        at android.app.ActivityThread.main(ActivityThread.java:5268)
        at java.lang.reflect.Method.invoke(Native Method)
        at java.lang.reflect.Method.invoke(Method.java:372)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:902)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:697)
     Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'void android.widget.TextView.setText(java.lang.CharSequence)' on a null object reference
        at com.yozhik.myProjectName.view.fragments.MyFinalTermsFragment.setupMyInformation(MyFinalTermsFragment.java:145)
        at com.yozhik.myProjectName.view.fragments.MyFinalTermsFragment.setupWithData(MyFinalTermsFragment.java:133)
        at com.yozhik.myProjectName.view.activity.MyFinalActivity.onDataLoaded(MyFinalActivity.java:742)
        at com.yozhik.myProjectName.presenter.MyFinalPresenter$1.onNext(MyFinalPresenter.java:55)
        at com.yozhik.myProjectName.presenter.MyFinalPresenter$1.onNext(MyFinalPresenter.java:47)
        at io.reactivex.internal.operators.observable.ObservableObserveOn$ObserveOnObserver.drainNormal(ObservableObserveOn.java:200)
        at io.reactivex.internal.operators.observable.ObservableObserveOn$ObserveOnObserver.run(ObservableObserveOn.java:252)
        at io.reactivex.android.schedulers.HandlerScheduler$ScheduledRunnable.run(HandlerScheduler.java:109)
        at android.os.Handler.handleCallback(Handler.java:739) 
        at android.os.Handler.dispatchMessage(Handler.java:95) 
        at android.os.Looper.loop(Looper.java:135) 
        at android.app.ActivityThread.main(ActivityThread.java:5268) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at java.lang.reflect.Method.invoke(Method.java:372) 
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:902) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:697) 
05-11 18:11:07.953 2155-3877/? E/WifiStateMachine: Did not find remoteAddress {192.168.200.1} in /proc/net/arp
05-11 18:11:07.966 2155-3877/? E/WifiStateMachine: WifiStateMachine CMD_START_SCAN source -2 txSuccessRate=3800.62 rxSuccessRate=4732.06 targetRoamBSSID=any RSSI=-68
05-11 18:11:07.967 2155-3877/? E/WifiStateMachine: WifiStateMachine L2Connected CMD_START_SCAN source -2 2324, 2325 -> obsolete
05-11 18:11:08.021 2155-3896/? E/ConnectivityService: Unexpected mtu value: 0, wlan0
05-11 18:11:08.579 13514-13366/? E/WakeLock: release without a matched acquire!

enter image description here enter image description here enter image description here enter image description here

Fragment which is crashing in method setupData because data_1_txt is NULL sometimes.

public class MenuFragment1 extends Fragment {

    public SomeCustomData transferedDataFromActivity;
    private TextView data_1_txt;

    public static MenuFragment1 newInstance(SomeCustomData data) {
        Log.d("TEST", "MenuFragment1.newInstance");
        MenuFragment1 fragment = new MenuFragment1();

        Bundle args = new Bundle();
        args.putSerializable("DATA_FROM_ACTIVITY", data);
        fragment.setArguments(args);

        return fragment;
    }

    @Override
    public void onCreate(@Nullable Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        Log.d("TEST", "MenuFragment1.onCreate");

        if (getArguments() != null) {
            this.transferedDataFromActivity = (SomeCustomData) getArguments().getSerializable("DATA_FROM_ACTIVITY");
        }
    }

    @Override
    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
        Log.d("TEST", "MenuFragment1.onCreateView");
        View v = inflater.inflate(R.layout.menu_fragment_1, container, false);
        data_1_txt = (TextView) v.findViewById(R.id.data_1_txt);

        setupInOnCreateView();

        return v;
    }

    protected void setupInOnCreateView() {
        Log.d("TEST", "MenuFragment1.setupInOnCreateView");
        //initialization of all view elements of layout with data is happens here.
        setupData(transferedDataFromActivity);
    }

    public void setupData(SomeCustomData data) {
        Log.d("TEST", "MenuFragment1.setupData");
        this.transferedDataFromActivity = data;
        if (transferedDataFromActivity != null) {
            data_1_txt.setText(transferedDataFromActivity.Name);
        }
    }
}

Fragment layout:

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:background="@color/green"
    android:orientation="vertical">

    <TextView
        android:id="@+id/data_1_txt"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:background="@color/yellow"
        android:text="Test"
        android:textSize="20sp" />

    <include layout="@layout/description_layout" />

</LinearLayout>
yozhik
  • 4,644
  • 14
  • 65
  • 98
  • 1
    In your fragment you're actually doing `findViewById` in `onCreateView`. Try instead doing `findViewById` in your `setupData` method (as opposed to using the variable). If it's still null, it means that your fragment was destroyed. BTW, you need to be extra careful using `Thread` for this exact reason - it's not lifecycle aware – Dennis K May 14 '18 at 08:00
  • have you tried out my answer? just replace `getMyData()` with your `textview.setText("your string")` – Pouya Danesh May 14 '18 at 10:35
  • @pouya I did not tryid your answer yet - because it looks like hack for me, and I think the reason of this crash is something different, and also the reason is that this bug is highly hard to reproduce, so I can't reproduce it and tell you that is it fixed or no. But thanks anyway. – yozhik May 14 '18 at 11:20
  • @DennisK What do you mean about using Thread? I'm not using it now in code. And about "findViewById" according to documentation - onCreateView is the place to look for views and remember them in variables. And it's hard to reproduce this bug. – yozhik May 14 '18 at 11:24
  • @yozhik yes in fact it is a hack, and a pretty good one for that matter. but can you please post the code of your `FragmentManager` and the place you call all these in your `activity`? – Pouya Danesh May 14 '18 at 11:56
  • @pouya yes, the simple project link is in question description: https://github.com/yozhik/Reviews/tree/master/app/src/main/java/com/ylet/sr/review – yozhik May 14 '18 at 12:44
  • How many fragments do you have in the viewpager that is in production? – Harikumar Alangode May 17 '18 at 06:57
  • @HarikumarAlangode only two fragments. That source code example exacltly showing how I organize work on production code. – yozhik May 17 '18 at 07:53
  • your error log clearly shows you are getting null text in textview : ` Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'void android.widget.TextView.setText(java.lang.CharSequence)' on a null object reference ` debug it surely you will find an answer. – Stephen May 18 '18 at 05:13
  • @Stephen I know that)) But can't find steps to reproduce it.. – yozhik May 21 '18 at 08:57

3 Answers3

2

in my experience if there is an error in fragments it is usually because of pre-loading of fragments in viewpager and TabMenu so what I did and Suggest you do to is to check if the fragment is visible to user and if they were, get data and other things so here is my code:

public class Fragment1 extends Fragment {
boolean visible = false;
public static Fragment1 newInstance() {
    return new Fragment1();
}
@Override
public View onCreateView(LayoutInflater inflater,
                         ViewGroup container, Bundle savedInstanceState) {
    if (visible && isResumed()) {
        onResume();
    } else if (visible) {
        getMyData();
    }
    return rootView;
}
@Override
public void setUserVisibleHint(boolean isVisibleToUser) {
    super.setUserVisibleHint(isVisibleToUser);
    visible = isVisibleToUser;
    if (isVisibleToUser) {
        getMyData();
    }
    else {
    }
}

@Override
public void onViewCreated(View view, @Nullable Bundle savedInstanceState) {
    super.onViewCreated(view, savedInstanceState);
}
}

this way if the view is not created yet and is not visible to user fragment won't do anything .

hope this helps.

Pouya Danesh
  • 1,557
  • 2
  • 19
  • 36
  • 1
    @azizbekian i actually searched a lot in order to overcome the problem of pre loading fragments in viewpager but this was the best I came up with. if you have a better way please in light me. in my case my webservices were called all together. – Pouya Danesh May 14 '18 at 10:33
  • @azizbekian yes, please, if you have better solution - please share with us, thanks. – yozhik May 14 '18 at 11:25
  • You should do an additional check in `setUserVisibleHint()` of `getActivity() != null`, because if you log calls to `setUserVisibleHint`, you will see that it is being called many a times with value true, but if you also log `getActivity()`, you will see that when `setUserVisibleHint` is true, many a times `getActivity()` may return null. – Kruti Parekh May 16 '18 at 04:26
  • @parekhkruti26 what kind of check is better than seeing it in action, and I have done that and this works. and `getMyData()` is only called once. that is because i have checked the visibility to user and it is only visible if the activity is not null. further more I am using this code. it might be a hack yes, but it is working fine for me. with no errors – Pouya Danesh May 26 '18 at 20:03
2

I'm pretty sure your issues are due to keeping the references to fragments in an Array. Fragments have lifecycles and the references are not guaranteed to persist. As you said, it's hard to reproduce and track down exactly what's going wrong, but maybe you don't need to. Some suggestions on how to fix this:

  1. Do not store the references to fragments. Pretty much follow the example on Google's page (https://developer.android.com/training/animation/screen-slide) and instantiate a new fragment every time it's requested.

  2. If you are worried about performance and caching is solving it, try using FragmentStatePagerAdapter - it caches pages and manages fragments' states.

  3. If you need to access page fragments from the main fragment (or the activity), instead of storing references, use `findFragmentByTag' which will always return the currently active instance of the fragment.

Dennis K
  • 1,828
  • 1
  • 16
  • 27
  • Thanks for answer, I also have this thought, but on stackoverflow I also found that it's ok to keep referance for the fragment. Because I was wondering why I must find fragment always with "findFragmentByTag" it seems to me like overhead. – yozhik May 15 '18 at 07:59
  • and by the way, how can I "findFragmentByTag" if in FragmentStatePagerAdaper - I can only supply fragments with new MyFragment? I don't mention any tag there, it's implemented in the core of this adapter. And I need to somehow set new fresh data to that fragments, not recreate them. – yozhik May 17 '18 at 09:28
  • 1
    This is at least part of the problem so it gets my vote - if you look in the OP's linked Github repo [here](https://github.com/yozhik/Reviews/blob/2ee95a09cea0d1dd51eb3733b72cf827a8e6b229/app/src/main/java/com/ylet/sr/review/CollapsingActivity.java#L48) it seems there is manual caching of the Fragments as fields of the Activity. You should almost never do this - the Android OS manages the Fragments. Rolling your own adapter for the Fragments will probably get you into trouble like the current one ;-) – David Rawson May 20 '18 at 06:25
1

I strongly believe that things aren't that complicated as others explained. If i understand it correctly, the delay caused by the network transaction is the culprit.

Consider this scenario.

  • You are making a network request which makes some changes in the view at the end.
  • You switch the pager. (Fragment is detached, views are destroyed)
  • Here comes the callback from the network request. Guess what! A crash.

So when dealing dealing with views of fragments, it's always a good idea to be more careful. I usually do this.

//in the base class
protected boolean isSafe()
    {
        return !(this.isRemoving() || this.getActivity() == null || this.isDetached()
                || !this.isAdded() || this.getView() == null);
    }

//usage in derived classes
onNewtworkResult(Result result) {
    if(!isSafe())
        return;
    //rest of the code
}

Alternatively, You can wrap the potential code in a try catch too. But that is more like a blind shot (At least in this case).

Bertram Gilfoyle
  • 9,899
  • 6
  • 42
  • 67