Changes between Version 1 and Version 2 of Ticket #15574, comment 14


Ignore:
Timestamp:
2017-12-04T17:21:30+01:00 (8 years ago)
Author:
cmuelle8

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #15574, comment 14

    v1 v2  
    22
    33EDIT: added fixes to findbug rants introduced with r13191, see [https://josm.openstreetmap.de/jenkins/job/JOSM/3595/findbugsResult/new/].  There are also warnings about the usage of {{{System.gc()}}}, do you think its ok to work with {{{@SupressFBWarning ..}}} in these cases to ignore them?  They should be called at times, when performance is degraded anyway due to error or caught outOfMem condition.
     4
     5EDIT2: include proper checks in the event image dimensions cannot be read (because of file format or other condition), this ensures that the thread will not linger around waiting forever.  if lots of such images are loaded and switched between these lingering threads may aggregate, so not waiting forever and properly let them finish is somewhat important.
    46{{{
    57  #!patch
    … …  
    3537      * Manage the visible rectangle of an image with full bounds stored in init.
    3638      * @since 13127
    37 @@ -230,8 +239,8 @@
    38                  ((infoflags & ImageObserver.HEIGHT) == ImageObserver.HEIGHT)) {
     39@@ -216,8 +225,8 @@
     40 
     41         private final File file;
     42         private final int orientation;
     43-        private int width;
     44-        private int height;
     45+        private int width = 0;
     46+        private int height = 0;
     47 
     48         LoadImageRunnable(File file, Integer orientation) {
     49             this.file = file;
     50@@ -231,7 +240,7 @@
    3951                 this.width = width;
    4052                 this.height = height;
    41 -                synchronized (this) {
     53                 synchronized (this) {
    4254-                    this.notify();
    43 +                synchronized (ImageDisplay.this) {
    44 +                    ImageDisplay.this.notifyAll();
     55+                    this.notifyAll();
    4556                     return false;
    4657                 }
    4758             }
    48 @@ -242,14 +251,14 @@
    49          public void run() {
     59@@ -243,19 +252,26 @@
    5060             Image img = Toolkit.getDefaultToolkit().createImage(file.getPath());
    5161 
    52 -            synchronized (this) {
    53 +            synchronized (ImageDisplay.this) {
    54                  width = -1;
     62             synchronized (this) {
     63-                width = -1;
    5564                 img.getWidth(this);
    5665                 img.getHeight(this);
    5766 
    58                  while (width < 0) {
     67-                while (width < 0) {
     68+                long now = System.currentTimeMillis();
     69+                while (!(width > 0 && height > 0)) {
    5970                     try {
    6071-                        this.wait();
    61 +                        ImageDisplay.this.wait();
    62                          if (width < 0) {
    63                              errorLoading = true;
     72-                        if (width < 0) {
     73-                            errorLoading = true;
     74+                        this.wait(1000);
     75+                        if (this.file != ImageDisplay.this.file)
    6476                             return;
    65 @@ -566,10 +575,12 @@
     77-                        }
     78+                        if (System.currentTimeMillis() - now > 10000)
     79+                            // no image dimensions could be read within timeout
     80+                            synchronized (ImageDisplay.this) {
     81+                                errorLoading = true;
     82+                                return;
     83+                            }
     84                     } catch (InterruptedException e) {
     85-                        e.printStackTrace();
     86+                        Logging.trace(e);
     87+                        Logging.warn("InterruptedException in {0} while getting properties of image {1}",
     88+                                getClass().getSimpleName(), file.getPath());
     89+                        Thread.currentThread().interrupt();
     90                     }
     91                 }
     92             }
     93@@ -279,24 +295,19 @@
     94                         Thread.sleep(5);
     95                     } catch (InterruptedException e) {
     96                         Logging.trace(e);
     97-                        Logging.warn("InterruptedException in "+getClass().getSimpleName()+
     98-                                " while loading image "+file.getPath());
     99+                        Logging.warn("InterruptedException in {0} while loading image {1}",
     100+                                getClass().getSimpleName(), file.getPath());
     101                         Thread.currentThread().interrupt();
     102                     }
     103                 }
     104                 if (tracker.isErrorID(1)) {
     105+                    // the tracker catches OutOfMemory conditions
     106                     img = null;
     107-                    System.gc();
     108                 }
     109             } else {
     110                 img = null;
     111             }
     112 
     113-            if (img == null || width <= 0 || height <= 0) {
     114-                tracker.removeImage(img);
     115-                img = null;
     116-            }
     117-
     118             synchronized (ImageDisplay.this) {
     119                 if (this.file != ImageDisplay.this.file) {
     120                     // The file has changed
     121@@ -566,10 +577,12 @@
    66122         public void mouseReleased(MouseEvent e) {
    67123             File file;