android-async-http / android-async-http

An asynchronous, callback-based Http client for Android built on top of Apache's HttpClient libraries.
https://github.com/android-async-http/android-async-http#this-project-is-no-longer-maintained-and-is-currently-deprecated-and-insecure-to-use
Apache License 2.0
10.63k stars 4.1k forks source link

JsonHttpResponseHandler throws NullPointerException when run in loop #397

Closed anulman closed 10 years ago

anulman commented 10 years ago

JsonHttpResponseHandler is throwing a NullPointerException when it is run in a loop. The standard AsyncHttpResponseHandler does not.

JsonHttpResponseHandler code:

public class RefreshLoopTask extends AsyncTask <String, String, String> {       
    @Override
    protected String doInBackground(String... nothing) {
        try {
            while(true) {
                String url = "http://echo.jsontest.com/key/value/one/two";
                AppRestClient.get(url, null, new JsonHttpResponseHandler() {
                    @Override
                    public void onSuccess(JSONObject res) {
                        Log.d("logger", "succ - " + res.toString());
                    }
                    @Override
                    public void onFailure(Throwable e, JSONObject res) {
                        Log.d("logger", "fail - " + res.toString());
                    }
                    @Override
                    public void onFinish() {
                        Log.d("logger", "finis");
                    }
                });
                Thread.sleep(5000);
            }
        } catch (InterruptedException e) {
            RefreshLoopTask t = new RefreshLoopTask();
            t.execute();
        }
        return null;
    }
}

Log output:

11-27 19:05:37.618: E/AndroidRuntime(20517): FATAL EXCEPTION: Thread-433
11-27 19:05:37.618: E/AndroidRuntime(20517): java.lang.NullPointerException
11-27 19:05:37.618: E/AndroidRuntime(20517):    at com.loopj.android.http.AsyncHttpResponseHandler.postRunnable(AsyncHttpResponseHandler.java:412)
11-27 19:05:37.618: E/AndroidRuntime(20517):    at com.loopj.android.http.JsonHttpResponseHandler$1.run(JsonHttpResponseHandler.java:153)
11-27 19:05:37.618: E/AndroidRuntime(20517):    at java.lang.Thread.run(Thread.java:841)

AsyncHttpResponseHandler code:

public class RefreshLoopTask extends AsyncTask <String, String, String> {

    @Override
    protected String doInBackground(String... nothing) {
        try {
            while(true) {
                String url = "http://echo.jsontest.com/key/value/one/two";
                AppRestClient.get(url, null, new AsyncHttpResponseHandler() {
                    @Override
                    public void onSuccess(String res) {
                        Log.d("logger", "succ - " + res);
                    }
                    @Override
                    public void onFailure(Throwable e, String res) {
                        Log.d("logger", "fail - " + res);
                    }
                    @Override
                    public void onFinish() {
                        Log.d("logger", "finis");
                    }
                });
                Thread.sleep(5000);
            }
        } catch (InterruptedException e) {
            RefreshLoopTask t = new RefreshLoopTask();
            t.execute();
        }
        return null;
    }
}

Log output:

11-27 19:08:22.008: D/logger(20757): GET - http://echo.jsontest.com/key/value/one/two
11-27 19:08:22.068: D/logger(20757): succ - {
11-27 19:08:22.068: D/logger(20757):    "one": "two",
11-27 19:08:22.068: D/logger(20757):    "key": "value"
11-27 19:08:22.068: D/logger(20757): }
11-27 19:08:22.072: D/logger(20757): finis
11-27 19:08:27.008: D/logger(20757): GET - http://echo.jsontest.com/key/value/one/two
11-27 19:08:27.080: D/logger(20757): succ - {
11-27 19:08:27.080: D/logger(20757):    "one": "two",
11-27 19:08:27.080: D/logger(20757):    "key": "value"
11-27 19:08:27.080: D/logger(20757): }
11-27 19:08:27.080: D/logger(20757): finis
anulman commented 10 years ago

Update: my workaround for now is to replicate the JsonHttpResponseHandler's parseResponse method as a static method in the AppRestClient class, then call it and cast its response accordingly.

Example, assumes existence of public static Object parseResponse(String r) on AppRestClient:

@Override
public void onSuccess(String res) {
    Log.d("logger", "succ - " + res);
    JSONObject jsonResponse = (JSONObject) AppRestClient.parseResponse(res);
    Log.d("logger", jsonResponse.toString();
}
smarek commented 10 years ago

I don't even see the line 412 in AsyncHttpResponseHandler, what version of library do you use? Anyway, I assume this method should be fixed, do you know what has been detected as null?

protected void postRunnable(Runnable runnable) {
    // Adding handler != null should avoid NPE but is not general solution
    if (runnable != null /*&& handler != null*/) {
        handler.post(runnable);
    }
}
anulman commented 10 years ago

No idea what's detected as null. Using 1.4.4, the corresponding lines are:

JsonHttpResponseHandler (153==first line):

postRunnable(new Runnable() {
    @Override
    public void run() {
        if (jsonResponse instanceof JSONObject) {
            onSuccess(statusCode, headers, (JSONObject) jsonResponse);
        } else if (jsonResponse instanceof JSONArray) {
            onSuccess(statusCode, headers, (JSONArray) jsonResponse);
        } else if (jsonResponse instanceof String) {
            onSuccess(statusCode, headers, (String) jsonResponse);
        } else {
            onFailure(new JSONException("Unexpected type " + jsonResponse.getClass().getName()), (JSONObject) null);
        }

    }
});

AsyncHttpResponseHandler (412==handler.post):

protected void postRunnable(Runnable r) {
    if (r != null) {
        handler.post(r);
    }
}

Looks like JsonHttpResponseHandler is having trouble with the super() init calls bubbling up through TextHttpResponseHandler, as Async declares handler on init; since Async works itself, I'd imagine it isn't a case of Looper.myLooper() returning null.

Any advice re: submitting a patch? Java is far from my primary language, though I'd be happy to help if possible.

smarek commented 10 years ago

Looks like handler got's released or nulled. Because the handler itself is WeakReference this can happen, I'll try to get more into your problem and see why it should be released mid-processing.

kokospapa8 commented 10 years ago

Hi, I have encountered the same issue and found out handler was never created in the beginning. because it checks if (Looper.myLooper() != null)

hosterzhu commented 10 years ago

Hi, I have encountered the same issue . Using 1.4.4 , How to fix it .

kokospapa8 commented 10 years ago

This issue occurred because I made the call on worker thread. When called the request on mainthread using( runonuithread) it worked fine.

hosterzhu commented 10 years ago

I use Handler sendMessage do some request . OK

lucasjotab3 commented 10 years ago

same issue here, how did you fix it? even if using runonuithread I still get this error.

hosterzhu commented 10 years ago

I send a message. In handler do your request.

发自 Windows 邮件

发件人: lucasjotab3 发送时间: ‎2013‎年‎12‎月‎3‎日 ‎2‎:‎58 收件人: loopj/android-async-http 抄送: hosterzhu 主题: Re: [android-async-http] JsonHttpResponseHandler throws NullPointerException when run in loop (#397)

same issue here, how did you fix it?

— Reply to this email directly or view it on GitHub.

hosterzhu commented 10 years ago

Why you using in a workthread ? I use workthread do complte send a message return mainthread do request .

发自 Windows 邮件

发件人: nujabes8 发送时间: ‎2013‎年‎12‎月‎2‎日 ‎13‎:‎25 收件人: loopj/android-async-http 抄送: hosterzhu 主题: Re: [android-async-http] JsonHttpResponseHandler throws NullPointerException when run in loop (#397)

This issue occurred because I made the call on worker thread. When called the request on mainthread using( runonuithread) it worked fine.

— Reply to this email directly or view it on GitHub.

lucasjotab3 commented 10 years ago

I'm not sure if I got it, please take a look at my code below:

this is where I make the request, inside some fragment's onCreateView... I'm not using asynctask or worker threads so far

RequestParams params = getCurrentCoordenates();
    params.put("categoryId", cat);
    AppAPI.get(getActivity(), "/listing/nearbydatatree.php", params, new JsonHttpResponseHandler() {
        @Override
        public void onSuccess(JSONObject r) {
            Crashlytics.log(TAG+" - "+"downloadJson() retornou onSuccess.");
            didDownload(r);
        }

        @Override
        public void onFailure(Throwable error, String content) {
            Crashlytics.log(TAG+" - "+"downloadJson() retornou onFailure. Erro: "+String.valueOf(error)+". Content: "+content);
            didDownloadWithError(content);
            if (error!=null){
                Crashlytics.logException(error);
            }
        }
    });

AppAPI.get is a static method from a helper class I use to organize the requests:

public static void get(Context c, String url, RequestParams params, AsyncHttpResponseHandler responseHandler) {
    context = c;
    if (isNetworkAvailable()) {
            client.setUserAgent(getDefaultUserAgent());
            client.get(getAbsoluteUrl(url), params, responseHandler);

    } else {
    responseHandler.onFailure(null, noConnection);
    }
}

when debugging, I find myself inside this Looper.java loop() function (a for loop at line 123). Somewhere inside it my app crashes... When debugging it seemed to be inside a infinite loop, though...

log: 12-03 09:35:13.743: ERROR/AndroidRuntime(20100): FATAL EXCEPTION: Thread-1627 Process: br.com.maisapp, PID: 20100 java.lang.NullPointerException at com.loopj.android.http.AsyncHttpResponseHandler.postRunnable(AsyncHttpResponseHandler.java:412) at com.loopj.android.http.JsonHttpResponseHandler$1.run(JsonHttpResponseHandler.java:153) at java.lang.Thread.run(Thread.java:841)

if I try this (whith donwloadJson() being the method that makes the request) getActivity().runOnUiThread(new Runnable() { public void run() { downloadJson(); } }); Nothing seems to change

Osram-Lux commented 10 years ago

I have encountered the same error:

12-04 19:35:46.141    1323-1497/com.example.test E/AndroidRuntime﹕ FATAL EXCEPTION: Thread-55
    java.lang.NullPointerException
            at com.loopj.android.http.AsyncHttpResponseHandler.postRunnable(AsyncHttpResponseHandler.java:412)
            at com.loopj.android.http.JsonHttpResponseHandler$1.run(JsonHttpResponseHandler.java:153)
            at java.lang.Thread.run(Thread.java:1019)

I suspect that this is some kind of a RaceCondition that occurs in JsonHttpResponseHandler, since I'm not running "AppRestClient.get(...)" in a loop. Possibly the "AppRestClient.get(...)" call needs to be done on the UI thread to avoid problems (and if it is a race-condition the other response handlers could have the same disease, but no symptoms).

Unfortunately the line numbers are off to investigate by just looking at the source (there is no line 412 in https://github.com/loopj/android-async-http/blob/master/library/src/main/java/com/loopj/android/http/AsyncHttpResponseHandler.java - can we thank ProGuard for this or is there something strange going on?)

dambrisco commented 10 years ago

Ran into this issue myself and here's what I've come up with:

The line numbers correlate with 375c4cb9f312c40fe536c84606108d0405e1b01a. As stated above in this thread, it looks like handler is ending up null and causing this error under certain conditions.

A primary condition for this appears to be creating an asynchronous call from a thread that does not have it's own looper. This will prevent handler from initializing, thereby causing a NullPointerException when handler is used to call Runnable r on line 412 of AsyncHttpResponseHandler in commit 375c4cb9f312c40fe536c84606108d0405e1b01a

For a shitty workaround, you can extend your preferred ResponseHandler using something like:

@Override
protected void postRunnable(Runnable r) {
    new Thread(r).run();
}

This will NOT run your calls on the same thread it originated from, so I would advise finding another solution if you need to run your code on the UI thread. Additionally, this will require you to ensure that any code which requires a looper to call Looper.prepare() on it's own as there won't be a Looper attached to the code.

A better workaround would almost certainly be something like:

@Override
protected void postRunnable(Runnable r) {
    boolean hasLooper = true;
    if (Looper.myLooper() == null) {
        Looper.prepare();
        hasLooper = false;
    }
    Handler handler = new AsyncHttpResponseHandler.ResponderHandler(this);
    handler.post(r);
    if (!hasLooper)
        Looper.loop();
}

but this would require modification of the library itself, specifically the addition of a public or protected modifier onto the internal ResponderHandler class.

Osram-Lux commented 10 years ago

Thanks for the input dambrisco

After inspecting my own code I think that i have managed to call AppRestClient.get(...) on a thread without a Looper (there is a code path from a timer task). I have added the following code in a strategic place:

if (null == Looper.myLooper()) {
    Log.e(TAG, "onLocationChanged runs on a thread without a looper - AsyncHttpResponseHandler.java will cast a Null Pointer exception");
}

If it fires and I get the exception, I think that the problem has been verified. My solution will then be to ensure that the data load code is run on the UI thread, as is the case for the majority of the calls.

I will report back when there are results (I only get the problem intermittently, probably when timer triggers and i have to realod data)

Osram-Lux commented 10 years ago

After changing my code to trigger the timeout consistently, I could reproduce the error, and indeed, the timer thread did not have a looper. Wrapping the call to AppRestClient.get(...) in a runOnUITread:

this.getActivity().runOnUiThread(new Runnable() {
    @Override
     public void run() {
         AppRestClient.get(...) 
     ...

did remove the exception.

I think that the constructor in AsyncHttpResponseHandler.java:168 should be amended like this:

public AsyncHttpResponseHandler() {
    // Set up a handler to post events back to the correct thread if possible
    if (Looper.myLooper() != null) {
        handler = new ResponderHandler(this);
    } else {
        throw new RejectedExecutionException("AsyncHttpClient can not run on a thread without a Looper (see: http://developer.android.com/reference/android/os/Looper.html)") ;
}

If AsyncHttpClient must run on the UI thread, check and throw as follows:

public AsyncHttpResponseHandler() {
    // Set up a handler to post events back to the correct thread if possible
    if (Looper.myLooper() == Looper.getMainLooper()) {
        handler = new ResponderHandler(this);
    } else {
        throw new RejectedExecutionException("AsyncHttpClient must run on the UI thread (see: http://developer.android.com/reference/android/app/Activity.html#runOnUiThread(java.lang.Runnable)l)") ;
}

If it is OK not to run on the UI thread and it is OK to add an looper implicitly, the constructor code could be changed as follows:

public AsyncHttpResponseHandler() {
    // Set up a Looper if thread does not have one
    if (Looper.myLooper() == null) {
        Looper.prepare();
    }
    // Set up a handler to post events back to the correct thread
    handler = new ResponderHandler(this);

Since I do not know which precondition that applies, I hope one of the maintainers could take a look at this and make a wise decision.

vekexasia commented 10 years ago

I'm also having issues related to - possibly - weakreference.

In my case i do .get with an instance of JsonHttpResponseHandler. The request starts as my onStart method gets called. then, in some rare cases, i don't get any other callback called: no onSuccess, no onFailure, no onFinish.

This bug happes only when i launch the my app and android takes a bit of time to render it. I believe some memory stuff needs to get cleared and the weakreference gets lost.

for now I fixed it by using an new Handler().postDelayed(new Runnable(){}, 500)

but this is a workaround. I'm not expert of using WeakReference but what is the mean of using it if in certain conditions the request gets dropped in the middle of nowhere?

smarek commented 10 years ago

@dambrisco I like your work on this case, using custom Looper if the library is not used from thread using one. I can try to implement it, or could provide a pull request, I'll see what will be done earlier. Same applies to investigation done by @Osram-Lux , thank you guys

smarek commented 10 years ago

@anulman @Osram-Lux @dambrisco @lucasjotab3 please test provided fix, test against a3eb0534cab8f12821d812efeff11f30760d0bb9 please. Possibly close this issue, if the latest master code is solution, and can be merged into next version of library.

dambrisco commented 10 years ago

@smarek New fix appears to be working correctly. I'd consider this issue closed with your next release.

dambrisco commented 10 years ago

I may have pre-emptively stated that the fix was working correctly, at least in all contexts. There appears to be an issue with the runnable never actually running under certain conditions. I'm still working out what exactly the reason is, but given that using my new Thread(r).run() override fixes the issue I'd have to gamble that it's a blocking issue with the Looper.

I'll open a new issue when I have some more time as it's not directly related to this issue, but I wanted to make anyone interested aware that this is a known problem as of a3eb0534cab8f12821d812efeff11f30760d0bb9.

carlosefonseca commented 10 years ago

Shouldn't the looper be quitted eventually? I'm ending up with a few Running threads but I'm not figuring out how to quit the looper…

Edit: Well, creating a new instance of AsyncHttpResponseHandler on a thread that's not the UI thread makes the constructor not return because of the looper.

smarek commented 10 years ago

@dambrisco thanks, I'll be waiting to hear from you, I'm also trying to simulate the test case where the issue happens.

@carlosefonseca can you add some code and start a new issue please? Thanks

kapsolas commented 10 years ago

I am encountering this issue. I am using the loopj library in the network.

Can someone help on pointing me in the direction for fixing this?

I have an object Client that exposes a method that then fires the request. The handler is a JsonHttpResponseHandler that processes the request.

The client is triggered in the runOnUIThread.

Thanks!

tagrudev commented 10 years ago

I had the same issue with JsonResponseHandler, works fine with AsyncHttpResponseHandler though

AAverin commented 10 years ago

Looks like this one is still alive. I have 1.4.4 lib version via gradle and get the same null pointer when I attempt to run any request in a separate thread that doesn't have a looper - try running a request in a TimerTask.

suneets commented 10 years ago

I'm hitting the same issue as well with version 1.4.4 lib bersion via gradle.. Any update?

socrateslee commented 10 years ago

The milestone is with version 1.4.5.

gongzunpan commented 10 years ago

印象笔记无法提交笔记,原因如下:

本月帐户上传流量已经达到上限。

原消息详情: 来自:Li <notifications@github.com> 发送到:gongzunpan.e099425@m.yinxiang.com 全部收件人:loopj/android-async-http <android-async-http@noreply.github.com> 主题:Re: [android-async-http] JsonHttpResponseHandler throws NullPointerException when run in loop (#397)

为了防止邮件过多,接下来的360分钟内,你将不会收到报错回复。

升级到印象笔记高级帐户,可以发送的邮件数量将从50封提升到200封。 https://app.yinxiang.com/Checkout.action?origin=email%2Dcommerce