0

After clicking a button in my app, it creates new normal Thread and start downloading large image and saving it to file. Everything is going well, but when i click button more than once it's going without errors and when i try to view these images they're bugged like they re overwriting themself.

I don't have any idea how to debug it.

localPath = today + "/" + productCode + "/" + this.placeId; //Unique

/* ... */

private void productSave(String productCode, int whichCamera, boolean isError) {
  for (int i = position; i < lastCamera; i++) {
    Date dateSave = new Date();
    path = localPath + "/" + dateFormat.format(dateSave) + "_" + (i + 1) + ".jpg";

    try {
      BufferedImage imageOld = ImageIO.read(new URL(this.camerasUrlsToSave[i]));

      ImageIO.write(imageOld, "jpg", new File(rootPath + "/" + path));

      ComDb.getInstance().saveProduct(productCode, this.placeId, path, dateSave);
    } catch (IOException ex) {
      result = false;
    }
  }
}

EDIT: path is 100% unique (different folders with product code). And it shoudn't be problem with image from camera - I can open 10 cards i dont see image bugs

EDIT2: Can it be something like downloading Img bufor? Cause all images are downloaded from the same IP. Or maybe its problem with bufferedimg memory leaks. Need idea how to repair it.

EDIT3: I found that if i open 5 cards in web browser with my camera address like : blah.blah.some.ip/GetImage.cgi?CH=0 They're loading one after the other, not all at once. But, i dont see bugged images when downloading ends.

EDIT4: I tried to reproduce this bug in web browser, if i try to open link in ff and in IE. IE prints "getImage busy". When I try ff and chrome i got broken images. So i have to do sth like queue or disable button ...

EDIT5: My temporary solution: synchronized function productSave. Images from second click will be saved few seconds later.

http://oi57.tinypic.com/ofrrn.jpg!

One from saved Images

szymon Hab
  • 11
  • 1
  • 4
  • where is variable i coming from? is this full code? – tgkprog Aug 29 '14 at 12:42
  • 1
    possible duplicate of [Threads and file writing](http://stackoverflow.com/questions/9972549/threads-and-file-writing) – BCartolo Aug 29 '14 at 12:45
  • @BCartolo every image has its own file. – szymon Hab Aug 29 '14 at 12:56
  • @szymon Hab If the user clicks on the button multiple times, then the method will be called several times with the same arguments. Am I right? Otherwise gives us more context. – BCartolo Aug 29 '14 at 12:57
  • Bonus tip: Creating a new 3000 x 3000 `BufferedImage` of type INT is going to cost you 36000000 bytes (and some time allocating that huge array). But it won't give you anything, as the next line will overwrite. This is not your main problem, though. – Harald K Aug 29 '14 at 12:58
  • What is `position` and `lastCamera`? I bet you that the problem is that your algorithm for creating file names doesn't create unique names. Try using a random UUID instead. – Harald K Aug 29 '14 at 13:00
  • @haraldK position is first work position camera id, lastCamera is lastCamera id in actual work position – szymon Hab Aug 29 '14 at 13:29
  • @szymonHab That doesn't tell me anything. Instead update the code in your question to make it clear. – Harald K Aug 29 '14 at 13:30
  • @BCartolo but path will contain different product ID – szymon Hab Aug 29 '14 at 13:31

2 Answers2

1

First action of the click event for the button should be to disable the button and maybe change the text to "In process". Last action should be to re-enable the button and restore the text.

BCartolo
  • 720
  • 4
  • 21
  • then what is the point of using threads? just keep variables seperate – tgkprog Aug 29 '14 at 12:50
  • The point is not to block the GUI thread. http://blogs.planetsoftware.com.au/paul/archive/2010/12/05/waiting-for-a-task-donrsquot-block-the-main-ui-thread.aspx – BCartolo Aug 29 '14 at 12:53
  • I cant disable button because people must have possibility to save next image even if previous one isn't downloaded yet – szymon Hab Aug 29 '14 at 13:05
0

This answer is a guess as you have not given full code, issue could be with variable i - where is it from?

Or could be same file name is being reused, mnake sure that is nto the case by getting unique file name from a seperate function something like this:

if dateFormat is only to minute or second, same file name might be used for 2 images use this API of java.io.File to get a unique name

http://docs.oracle.com/javase/7/docs/api/java/io/File.html#createTempFile%28java.lang.String,%20java.lang.String,%20java.io.File%29

public static File createTempFile(String prefix, String suffix, File directory)

//you can pass extn as jpg

public File getFileName(File localPath ,Date dateSave, int i, String extn){
    File fileUniqe = File.createTempFile(dateSave + "_" + (i+1), extn, localPath );
    return fileUniqe,
}
tgkprog
  • 4,493
  • 4
  • 41
  • 70
  • so i is a local variable, still use above function to get a unique file name – tgkprog Aug 29 '14 at 12:50
  • Updated code which im already debugging, Its not a problem with filename - its 100% unique because localpath points current product code – szymon Hab Aug 29 '14 at 12:52
  • what is the harm in trying this? – tgkprog Aug 29 '14 at 20:37
  • I added next variable to my filename like this, but it didn't help. I started thinking about IP camera because i don't see any similar problem and solution in Java. – szymon Hab Aug 30 '14 at 06:45