ConcurrentModificationException when using addOnMixpanelUpdatesReceivedListener

[dbarwacz]

Unregistering consumed listener from OnMixpanelUpdatesReceivedListener#onMixpanelUpdatesReceived callback sometimes causes ConcurrentModificationException. There is no easy way for the client to unregister listener without risking crash. Reson: MixpanelAPI:2111 (4.9.2) - notifying listeners on the loop that directly stores registered listeners.

[patedit]

@dbachelder I opened a PR #397
Hopefully it will solve your issue. In the mean time, do you have a way to reproduce this issue?

[dbarwacz]

Sorry for the delay.
I'm afraid synchronising won't change much but I'm no java concurrency expert. I will create empty project with reproductible error (should be doable) within days.
I will need to use some api key probably, are there spare keys available publicly for test/dev purposes? I don't want to use our internal test keys and registering just for the key will probably be a waste of time.

[patedit]

@dbarwacz Let me know if you keep seeing this issue with 4.9.3 :) Thanks!

[dbarwacz]

@patedit Hi! Finally got around to create example project, sorry it took so long.
So, in order for the error to appear, you need to do the following:

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        MixpanelAPI mixpanel = MixpanelAPI.getInstance(this, YOUR_PROJECT_TOKEN);
        MixpanelAPI.People people = mixpanel.getPeople();
        Factory factory = new Factory(people);

        for (int i = 0; i < 100; i++) {
            people.addOnMixpanelUpdatesReceivedListener(factory.getListener());
        }
    }

Where factory is:

private static class Factory {
        private final MixpanelAPI.People people;

        public Factory(MixpanelAPI.People people) {
            this.people = people;
        }

        OnMixpanelUpdatesReceivedListener getListener() {
            return new OnMixpanelUpdatesReceivedListener() {
                @Override
                public void onMixpanelUpdatesReceived() {
                    Log.i("TAG", "onMixpanelUpdatesReceived");
                    people.removeOnMixpanelUpdatesReceivedListener(this);
                }
            };
        }
    }

The reason we see it so rarely is the fact that it requires strange conditions that we don't encounter normally (registering more than one listener).

Well, one more thing: token need to have a/b testing defined, I have one prepared so pm if you don't want to generate new one.

[patedit]

Sorry for the waiting - I've been pretty busy and I haven't been able to implement a proper fix. Your snippet is super useful and I could make it crash so we will have fix soon.

[dbarwacz]

Hey! How are we on this one? Can we expect release with fix soon?

[patedit]

@dbachelder Apologies again for the long wait. I've implemented a fix for your issue (#447) that will be release at the end of today (version 5.0.1). Thanks for your patience!