stephanenicolas / robospice

Repo of the Open Source Android library : RoboSpice. RoboSpice is a modular android library that makes writing asynchronous long running tasks easy. It is specialized in network requests, supports caching and offers REST requests out-of-the box using extension modules.
Apache License 2.0
2.95k stars 545 forks source link

Fatal Exception after cancel(request) [Bug] #48

Closed chrisjenx closed 11 years ago

chrisjenx commented 11 years ago

Kills the app when trying to or after cancelling the request. sometimes. Or if passing a null listener through to the execute command.

Stack:

ERROR/AndroidRuntime(13335): FATAL EXCEPTION: main
        java.lang.NullPointerException
        at com.octo.android.robospice.request.RequestProcessor$ResultRunnable.run(RequestProcessor.java:496)
        at android.os.Handler.handleCallback(Handler.java:725)
        at android.os.Handler.dispatchMessage(Handler.java:92)
        at android.os.Looper.loop(Looper.java:137)
        at android.app.ActivityThread.main(ActivityThread.java:5039)
        at java.lang.reflect.Method.invokeNative(Native Method)
        at java.lang.reflect.Method.invoke(Method.java:511)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:793)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:560)
        at dalvik.system.NativeStart.main(Native Method)
stephanenicolas commented 11 years ago

Which version of RoboSpice are you using. The stack trace doesn't match actual code state.

chrisjenx commented 11 years ago

Version 1.4.0

stephanenicolas commented 11 years ago

You'r right, I took the wrong class. Bug is being investigating. Supporting null listener was not part of the specs of the RequestProcessor. But after all, it should be possible as well.

Nevertheless, would you have a test for the bug to appear using cancel ?

chrisjenx commented 11 years ago

Because I'm struggling to create a mock version of the SpiceManager I have no test case to send you otherwise I would.

This is a work around for a ExecutorService:

private void requestImage(final UserPhoto photo, final boolean fullSize, OnPhotoLoadListener listener)
    {
        final String key = fullSize ? photo.getDisplayImageKey() : photo.getThumbnailImageKey(UserPhoto.MINI_THUMBNAIL_SIZE);
        final BizzbyApplication app = BizzbyApplication.get();
        final BitmapLruCache cache = app.getImageCache();
        final CacheableBitmapDrawable cached = cache.get(key);

        if (null != cached && cached.hasValidBitmap())
        {
            QLog.d("Got Bitmap: Size: [" + cached.getBitmap().getWidth() + "," + cached.getBitmap().getHeight() + "]");
            setImageDrawable(cached);
            if (null != listener)
                listener.onPhotoLoadFinished(cached.getBitmap());
        }
        else
        {
            // Means we have an object with an invalid bitmap so remove it
            if (null != cached)
                cache.remove(key);

            mCurrentRequest = new PhotoLoadExecutor(this, photo, cache, fullSize);
            if (listener == null)
                listener = new OnPhotoLoadListener()
                {
                    @Override
                    public void onPhotoLoadFinished(final Bitmap bitmap)
                    {
                    }
                };
            app.getExecutorManager().execute(mCurrentRequest, listener);
        }
    }

Note the if(listener == null) if you pass through null at this point it will FC when trying to deliver the result.

I agree Result listener wont be null in most cases, but here where I am offloading image resizing. Sometimes the result is set inline and there is no need for a listener.

stephanenicolas commented 11 years ago

Bug has been solved and a new test has been added. It is available on master branch and has been cherry picked to release branch for next release.

Thx for your help. Let's investigate the mocking problem now ;)

chrisjenx commented 11 years ago

Nice, cheers Stephan. On 16 Feb 2013 20:47, "stephanenicolas" notifications@github.com wrote:

Bug has been solved and a new test has been added. It is available on master branch and has been cherry picked to release branch for next release.

Thx for your help. Let's investigate the mocking problem now ;)

— Reply to this email directly or view it on GitHubhttps://github.com/octo-online/robospice/issues/48#issuecomment-13674419.