capacitor-community / photoviewer

⚡ Capacitor plugin to view table images with fullscreen and sharing capabilities.
MIT License
52 stars 25 forks source link

App in prod calls ANR when user exits from image viewer #48

Open ivanov84 opened 1 year ago

ivanov84 commented 1 year ago

Describe the bug App in production calls ANR when user exits from image viewer

To Reproduce Steps to reproduce the behavior:

  1. open image
  2. press back button in phone bottom
  3. repeat before it apprears again

Expected behavior no ANR, photo viewer is closed

Smartphone (please complete the following information):

Additional context I'm going to try catch that code in file ImageFragment.kt:

private fun postNotification() {
        var info: MutableMap<String, Any> = mutableMapOf()
        info["result"] = true
        info["imageIndex"] = startFrom
        NotificationCenter.defaultCenter().postNotification("photoviewerExit", info);
}

Screenshots image image

jepiqueau commented 1 year ago

@ivanov84 To do what you want : "closing the app" when a second back-button's press you must do this in your app i show it for the angular framework modify the app.component.ts

...
import { Platform } from '@ionic/angular';
import { App } from '@capacitor/app';
...
export class AppComponent {
  constructor(private platform: Platform) {
    this.platform.ready().then(async () => {
      this.platform.backButton.subscribeWithPriority(
        666666, () => {
          App.exitApp();
        });

    });
  }
}

This will do the job for android only

jepiqueau commented 9 months ago

@ivanov84 Did that solve the issue in Android. Waiting for your answer otherwise i will close the issue

ivanov84 commented 7 months ago

@jepiqueau No, still error with new photoviewer version: image

brickedsolutions commented 3 weeks ago

I was having the same issue. ChatGPT fixed it for me by doing following changes in NotificationCenter.java

The ConcurrentModificationException in your code is likely happening because you are modifying the registeredObjects collection while iterating over it, which is not allowed in Java.

There are two places where this issue could occur:

  1. removeAllNotifications(): You are removing entries from registeredObjects while iterating over it.
  2. postNotification(): You are iterating over list in postNotification(), which could be modified (either by removing or adding methods for notification) during iteration.

Fixing the removeAllNotifications Method

When you want to remove elements from a collection while iterating over it, the safest approach is to use the Iterator.remove() method.

Update the removeAllNotifications() method like this:

public synchronized void removeAllNotifications() {
    Iterator<Map.Entry<String, ArrayList<MyRunnable>>> entryIterator = registeredObjects.entrySet().iterator();
    while (entryIterator.hasNext()) {
        Map.Entry<String, ArrayList<MyRunnable>> entry = entryIterator.next();
        ArrayList<MyRunnable> value = entry.getValue();
        if (!value.isEmpty()) {
            removeMethodForNotification(entry.getKey(), value.get(0));
        }
        entryIterator.remove(); // Safe way to remove during iteration
    }
}

Fixing the postNotification Method

In postNotification(), you are iterating over the list of MyRunnable objects. To ensure that the list is not modified during iteration (which could trigger the ConcurrentModificationException), you should iterate over a copy of the list.

Update the postNotification() method like this:

public synchronized void postNotification(String notificationName, Map<String, Object> _info) {
    ArrayList<MyRunnable> list = registeredObjects.get(notificationName);
    if (list != null) {
        // Create a copy of the list to avoid concurrent modification
        ArrayList<MyRunnable> listCopy = new ArrayList<>(list);

        for (MyRunnable r : listCopy) {
            if (r != null) {
                r.setInfo(_info);
                r.run();
            }
        }
    }
}

Why These Changes Work:

In removeAllNotifications(), using Iterator.remove() ensures safe removal while iterating over the collection. In postNotification(), making a copy of the list (listCopy) ensures that any changes to the original list (such as adding or removing MyRunnable objects) will not affect the ongoing iteration, preventing a ConcurrentModificationException.

These changes should help prevent the ConcurrentModificationException you're seeing.

robingenz commented 3 weeks ago

@brickedsolutions Feel free to create a PR.