multithreading - Java program with multiple threads not working -


so, i'm having problem gui i'm designing java app renames files in given directory junk (just fun). main block of code behind all:

import java.io.file; import java.io.ioexception; import java.io.printwriter; import java.util.arraylist; import java.util.hashmap; import java.util.scanner;  import org.json.simple.jsonobject; import org.json.simple.parser.jsonparser; import org.json.simple.parser.parseexception;  /**  * class renaming files garbage names.  * methods static, hence private constructor.  * @author shadow hacker  */  public class renamefiles {     private static int renamedfiles = 0;     private static int renamedfolders = 0;     public static char thechar = '#';     public static arraylist<file> filewhitelist = new arraylist<>();      public static hashmap<file, file> revert = new hashmap<>();      public static int getrenamedfiles() {         return renamedfiles;     }      public static int getrenamedfolders() {         return renamedfolders;     }      /**      * methods static, hence private constructor.      */      private renamefiles() {         // private constructor, nothing do.     }      /**       * @param file file rename.      * @param renameto current value of name rename to.      * @return new value renameto.      */      private static string renamefile(file file, string renameto) {         (file whitelistedfile : filewhitelist) {             if (whitelistedfile.getabsolutepath().equals(file.getabsolutepath())) {                 return renameto;             }         }         if (new file(file.getparentfile().getabsolutepath() + "/" + renameto).exists()) {             renameto += thechar;             renamefile(file, renameto);         } else {             revert.put(new file(file.getparent() + "/" + renameto), file);             file.renameto(new file(file.getparent() + "/" + renameto));             if (new file(file.getparent() + "/" + renameto).isdirectory()) {                 renamedfolders++;             } else {                 renamedfiles++;             }         }         return renameto;     }      /**       * todo add exception handling.      * @param dir root directory.      * @throws nullpointerexception if can't open dir      */      public static void renameallfiles(file dir) {         string hashtags = character.tostring(thechar);         (file file : dir.listfiles()) {             if (file.isdirectory()) {                 renameallfiles(file);                 hashtags = renamefile(file, hashtags);             } else {                 hashtags = renamefile(file, hashtags);             }         }     }      public static void renameallfiles(string dir) {         renameallfiles(new file(dir));     }      /**      * uses revert hashmap change files orignal names,      * if user decides didn't want change names of files later.      * @param dir directory in search.      */      public static void revert(file dir) {         (file file : dir.listfiles()) {             if (file.isdirectory()) {                 revert(file);             }             revert.foreach((name, renameto) -> {                 if (file.getname().equals(name.getname())) {                     file.renameto(renameto);                 }             });         }     }      public static void revert(string dir) {         revert(new file(dir));     }      /**      * saves revert configs json file; can't use obj.writejsonstring(out)      * because file's tostring() method calls getname(), , want full      * paths.      * @param wheretosave file save config to.      * @throws ioexception      */      @suppresswarnings("unchecked")     public static void saverevertconfigs(string wheretosave) throws ioexception {         printwriter out = new printwriter(wheretosave);         jsonobject obj = new jsonobject();         revert.foreach((k, v) -> {             obj.put(k.getabsolutepath(), v.getabsolutepath());         });         out.write(obj.tojsonstring());         out.close();     }      /**      * warning - clears revert.      * can't use obj.putall(revert) because puts strings      * revert, , want files.      * todo add exception handling.      * @param wheretoload path file load.      * @throws parseexception if file can't read.      */      @suppresswarnings("unchecked")     public static void loadrevertconfigs(string wheretoload) throws parseexception {         revert.clear();         ((jsonobject) new jsonparser().parse(wheretoload)).foreach((k, v) -> {             revert.put(new file((string) k), new file((string) v));         });     }      /**      * static block here because program uses foreach      * loops, , don't want methods call them      * return errors.      */      static {         if (!(system.getproperty("java.version").startswith("1.8") || system.getproperty("java.version").startswith("1.9"))) {             system.err.println("must use java version 1.8 or above.");             system.exit(1);         }     }      /**      * though made gui this, still has complete command-line interface       * because reasons.      * @param argv[0] folder rename files in; defaults current directory.      * @throws ioexception       */      public static void main(string[] argv) throws ioexception {         scanner scanner = new scanner(system.in);         string accept;         if (argv.length == 0) {             system.out.print("are sure want proceed? potentially damage system! (y/n) : ");             accept = scanner.nextline();             scanner.close();             if (!(accept.equalsignorecase("y") || accept.equalsignorecase("yes"))) {                 system.exit(1);             }              renameallfiles(system.getproperty("user.dir"));         } else if (argv.length == 1 && new file(argv[0]).exists()) {             system.out.print("are sure want proceed? potentially damage system! (y/n) : ");             accept = scanner.nextline();             scanner.close();             if (!(accept.equalsignorecase("y") || accept.equalsignorecase("yes"))) {                 system.exit(1);             }              renameallfiles(argv[0]);         } else {             system.out.println("usage: renameallfiles [\033[3mpath\033[0m]");             scanner.close();             system.exit(1);         }         system.out.println("renamed " + (renamedfiles != 0 ? renamedfiles : "no") + " file" + (renamedfiles == 1 ? "" : "s")                 + " , " + (renamedfolders != 0 ? renamedfolders : "no") + " folder" + (renamedfolders == 1 ? "." : "s."));     } } 

as can see, of it's methods static. here (only partially completed) event handler class:

import java.io.file;  /**  * seperate class gui event handlers.   * calls methods renamefiles.  * renamefiles, methods static.  * @author shadow hacker  */  public class eventhandlers {     private static thread t;      /**      * reason in new thread can check      * if done or not (for 'cancel' option).      * @param dir root directory used renamefiles.renameallfiles.      */      public static void start(file dir) {         t = new thread(() -> {             renamefiles.renameallfiles(dir);         });         t.start();     }      /**      * @param dir root directory used renamefiles.revert(dir).      * @throws interruptedexception      */      public static void cancel(file dir) throws interruptedexception {         new thread(() -> {             while (t.isalive()) {                 // nothing do; waiting t end.             }             renamefiles.revert(dir);         }).start();     }      public static void main(string[] args) throws interruptedexception {         start(new file("rename"));         cancel(new file("rename"));     } } 

the problem i'm having when run revert renamefiles class works fine, while running multithreaded (we don't want handlers have wait method finish before reacting button press) eventhandlers class, revert dosn't work. have renamefiles being class static methods, or else? please help!

edit: @douglas, when run:

import java.io.file; import java.util.concurrent.countdownlatch; import java.util.concurrent.executorservice; import java.util.concurrent.executors;  /**  * seperate class gui event handlers.   * calls methods renamefiles.  * renamefiles, methods static.  * @author shadow hacker  */  public class eventhandlers {     private static  executorservice service = executors.newsinglethreadexecutor();     private static volatile countdownlatch latch;      /**      * reason in new thread can check      * if done or not (for 'cancel' option).      * @param dir root directory used renamefiles.renameallfiles.      */      public static void start(file dir) {         latch = new countdownlatch(1);         service.submit(() -> {             renamefiles.renameallfiles(dir);             latch.countdown();         });       }      /**      * @param dir root directory used renamefiles.revert(dir).      * @throws interruptedexception      */      public static void cancel(file dir) throws interruptedexception {         service.submit(() -> {              try {                 latch.await();             } catch (interruptedexception e) {                 e.printstacktrace();             }             renamefiles.revert(dir);         });     } 

the program runs forever, without terminating.

you have 2 major problems here.

first, sharing variables between threads. default variable handling in java has no guarantee 2 threads agree on value given variable has. can fix 1 giving each variable volatile modifier (note: can decrease performance, why it's not default).

second, have no mechanism in place guarantee thread execution order. written, entirely possible eventhandlers.main run cancel completion before renameallfiles call starts. possible renaming start, paused thread scheduler, cancel run beginning end, , renaming finish, or of bunch of other combinations. attempted t.isalive() check, redundant creation of yet thread in main means there's no guarantee t initialized before main thread gets there. unlikely valid spec possibility nullpointerexception line.

this second problem harder 1 fix in general, , primary reason working threads infamously difficult. fortunately particular problem simple case. instead of looping forever on isalive() check, create countdownlatch when start thread, count down when thread finishes, , await() in cancel. incidentally solve first problem @ same time without need volatile, because in addition scheduling coordination countdownlatch guarantees thread awaited on see results of done in thread counted down.

so, long story short, steps fix this:

  1. remove new thread in main , call start directly. start creates thread itself, there's no need nest inside thread.
  2. replace thread t countdownlatch.
  3. in start, initialize countdownlatch count of 1.
  4. in start, after initializing countdownlatch, executorservice calling executors.newsinglethreadexecutor(), , submit renameallfiles call it. instead of using thread directly. among other things, specification guarantees done before visible expected in new thread, , don't see such guarantee in documentation of thread.start(). it's got lot more convenience , utility methods.
  5. inside submit executorservice, after renaming, call countdown() on latch.
  6. after submit, call shutdown() on executorservice. prevent reusing same one, stops waiting indefinitely reuse never happen.
  7. in cancel, replace while loop call await() on latch. in addition memory consistency guarantee, improve performance letting system thread scheduler handle wait instead of spending cpu time on looping.

additional changes needed if want account multiple rename operations in same run of program.


Comments

Popular posts from this blog

javascript - jQuery: Add class depending on URL in the best way -

caching - How to check if a url path exists in the service worker cache -

Redirect to a HTTPS version using .htaccess -