Todd-Davies / ProgressWheel

A progress wheel for android, intended for use instead of the standard progress bar.
MIT License
2.64k stars 710 forks source link

Potential memory leak? #4

Closed AwesomelyAmazing closed 9 years ago

AwesomelyAmazing commented 11 years ago

If we start spin it will forever post messages to handler (to update view). Since there are always messages in queue gc won't be able to collect this handler and therefore won't collect view and context it's attached to. So the only way to avoid leak is to call stop spin manually, which is troublesome in some situations. Maybe view should override onDetachedFromWindow to stop message posting to handler, but save state, so on the next attach it will start spinning again.

AwesomelyAmazing commented 11 years ago

As a quick fix I made following changes in view in my project:

    private Handler spinHandler = new SpinHandler(this);

    private static class SpinHandler extends Handler {

        private final WeakReference<ProgressWheelView> viewRef;

        public SpinHandler(final ProgressWheelView view) {
            viewRef = new WeakReference<ProgressWheelView>(view);
        }

        /**
         * This is the code that will increment the progress variable
         * and so spin the wheel
         */
        @Override
        public void handleMessage(final Message msg) {
            final ProgressWheelView progressView = viewRef.get();
            if (progressView != null) {
                progressView.invalidate();
                if (progressView.isSpinning) {
                    progressView.progress += progressView.spinSpeed;
                    if (progressView.progress > 360) {
                        progressView.progress = 0;
                    }
                    progressView.spinHandler.sendEmptyMessageDelayed(0, progressView.delayMillis);
                }
            }
        }
    }

and:

@Override
    public void onAttachedToWindow() {
        super.onAttachedToWindow();
        setupBounds();
        setupPaints();
        invalidate();
        if (isSpinning) {
            spinHandler.sendEmptyMessage(0);
        }
    }

    @Override
    protected void onDetachedFromWindow() {
        super.onDetachedFromWindow();
        spinHandler.removeMessages(0);
    }

With static handler nothing will stop gc from collecting view, and when it gets collected handler will just stop sending messages to itself. When we get detached, but not yet gced we should stop messages so they won't lag the ui thread (since there could be a gridview with progresses and if each will send messages every now and then ui will lag)

Todd-Davies commented 11 years ago

Aleksandrs, that looks great. I can't see any problems with the code.

You should submit a pull request for review.

Thanks.

belen-mapplas commented 10 years ago

Hi guys,

I have problems in a Samsung Galasy S3 device. I start spinning the progressWheel without problems, and even stopping it, if the screen of the phone turns off, the mobile phone doesn't come back from that state.

I have also tried on a HTC One X and works like a charm.

Some ideas?