Please read this personal appeal

Discussion in 'Plugin Development' started by bergerkiller, Dec 13, 2011.

Thread Status:
Not open for further replies.
  1. Offline

    bergerkiller

    Stop accessing Bukkit-managed variables and methods from an async task or thread.

    I am getting extremely tired of concurrent modification exceptions occuring under my world/chunk_(un)load events and while looping through world entities in my plugins. Seriously, STOP starting async tasks which alter unsynchronized Bukkit information. I had to synchronize all of my code under a single lock object because of you. -.-
     
    fromgate, Inscrutable, robxu9 and 5 others like this.
  2. Offline

    Sleaker

    same goes for everyone that thinks getOnlinePlayers is thread-safe. It's not. stop using it in asynch tasks.
     
  3. Offline

    Mr Burkes

    Idk what that means, (noob here) explain please, so I don't make any mistakes. :O
     
  4. Offline

    Sagacious_Zed Bukkit Docs

    If you schedule an asynchronous task with bukkits task scheduler, you cannot use most methods within those tasks. This is because an asynchronous task is running on the different threads, and the methods are not meant for use from multiple threads.
    for example. if you have two atms for one bank, and two people make a withdrawal from the same account at the same time, a method that is not thread safe could end up giving both people the withdrawal but only deduct one withdrawal from the account.
     
    pyraetos likes this.
  5. Offline

    pyraetos

    Especially don't use an async task to change massive amounts of blocks to different materials every half a second... like I did.
     
  6. Offline

    Mr Burkes

    I use Java's threads for some of my methods... is that safe?
     
  7. Offline

    Abrupt

    I've got 99 problems but concurrency aint one
     
    DirtyStarfish likes this.
  8. Offline

    V10lator

    No! Use bukkits scheduler and syncTasks (as long as you're talking to the bukkit API/non-threadsave objects in your code)
     
  9. Offline

    tips48

    It depends. If your accessing anything in Bukkit, chances are NO.
     
  10. Offline

    Mr Burkes

    Why? I don't understand how that isn't safe ... :(
     
  11. Offline

    tips48

    Because if one plugin changes a value while another plugin is using it (only possible if one plugin is using ASync tasks) then it throws a ConcurrentModificationException.

    Just change your scheduler calls from asyn to sync. You literally only have to remove the a.
     
  12. Offline

    ThatBox

    And add the c! But yeah nice thread :p
     
  13. Offline

    Don Redhorse

    ok... as a "noob"... how do you use such a "bukkits scheduler and syncTasks" ?

    a code snippet would be nice.. :)
     
  14. Offline

    Coryf88

  15. Offline

    bergerkiller

    Code:
    final int x = 12;
    final int z = 43;
    final World world = Bukkit.getServer().getWorld("world1");
    final Location dest = world.getSpawnLocation();
    plugin.getServer().getScheduler().scheduleAsyncDelayedTask(plugin, new Runnable() {
        public void run() {
            Chunk c = world.getChunkAt(x, z); //wrong! if this chunk wasn't loaded, it will try to load it from an async thread. lots of code break here!
            for (Entity e : c.getEntities()) { //wrong!
                if (e instanceof Player) e.teleport(dest); //wrong!
            }
        }
    }, 0, 10);
    Code:
    public class MyThread extends Thread {
    
        public void run() {
            while (true) {
                for (World world : Bukkit.getServer().getWorlds()) { //wrong, world can be loaded
                    for (Entity e : world.getEntities()) { //wrong, entity can be added/removed
                        if (e instanceof Creeper) {
                            e.remove(); //allowed, only alters field.  (e.dead = true) Might be on the danger side though
                        }
                    }
                }
            }
        }
        private static MyThread thread;
        public static void init() {
            thread = new MyThread();
            thread.start();
        }
    }
     
    Don Redhorse likes this.
  16. Offline

    Don Redhorse

    thanks... I think this will help some people... keep in mind not everybody is as good as "you" and understands what "you" are talking about.
     
  17. Offline

    AbdulAbdalla

    Are events (like chat commands) raised from the main thread or do i have to sync that too?
     
  18. Offline

    bergerkiller

    @AbdulAbdalla any Bukkit function which can result in any sort of event has to be run on the main thread, always.
    You don't know what other plugins do in those events. If someone decides to use the BLOCK_PHYSICS event to spawn a new entity, this will cause great havoc to the server if a plugin causes to fire that event from another thread. And worst of it all is that everyone is going to blame the plugin that handles BLOCK_PHYSICS, but he can't do anything about it!

    player.sendmessage is thread-safe: the chat message packet is added to a queue, which is synchronized, and then sent to the player. It doesn't fire an event either.
     
  19. Offline

    AbdulAbdalla

    Allright, so i even need to sync a simple player.teleport() in my onCommand callback i guess =/
     
  20. Offline

    Ninjokin

    Question, if I call a method that I created, which executes a Bukkit method then run that created method on another thread, is that safe?
     
  21. Offline

    bergerkiller

    @AbdulAbdalla definitely, it fires the PLAYER_TELEPORT event too, other than removing/adding the entity if teleporting cross-world.

    @Ninjokin No. The location of the method does not matter to a Thread. I'll fix up an example so you get an idea.
     
  22. Offline

    Ninjokin

    The method that calls the bukkit method is in the main threa, just so you know.

    @bergerkiller
     
  23. Offline

    bergerkiller

    @Ninjokin Methods can't be in a certain thread. Methods (and variables) are managed by the Java Environment. All threads do is fire those methods/calls one by one.

    What you meant then is:
    'The method is called on the main thread'.
    Then you are free to use it. But if the method is called on another thread (or scheduled async task), then that's bad.

    Code:
        public class Test {
            public ArrayList<String> items = new ArrayList<String>();
            public void addItem(String item) {
                items.add(item);
            }
            public void removeItem(String item) {
                items.remove(item);
            }
            public List<String> getItems() {
                ArrayList<String> clone = new ArrayList<String>(items.size());
                clone.addAll(items);
                return clone;
            }
    
            private Thread addThread;
            private Thread remThread;
            private Thread loopThread;
            public void init() {
                final Test me = this;
                addThread = new Thread() {
                    int i = 0;
                    public void run() {
                        while (true) {
                            me.addItem("item" + i);
                            i++;
                            try {
                                Thread.sleep(500);
                            } catch (InterruptedException e) {}
                        }
                    }
                };
                remThread = new Thread() {
                    int i = 0;
                    public void run() {
                        while (true) {
                            me.removeItem("item" + i);
                            i++;
                            try {
                                Thread.sleep(500);
                            } catch (InterruptedException e) {}
                        }
                    }
                };
                loopThread = new Thread() {
                    public void run() {
                        while (true) {
                            for (String item : me.getItems()) {
                                System.out.println(item);
                            }
                            try {
                                Thread.sleep(500);
                            } catch (InterruptedException e) {}
                        }
                    }
                };
                addThread.start();
                remThread.start();
                loopThread.start();
            }
        }
    This is to give you an idea. Thread a adds an entity, Thread b removes an entity and Thread c acts as the main thread loop whichs loops through the entities. If you spawn a new entity (like Thread A does), this will cause the main loop to spit out concurrent modification exceptions. Even though I return a clone from that internal list!
     
  24. Offline

    AbdulAbdalla

    I use the bukkit database interface. Should i query / update data within the main thread too? Sometimes i got a persistence data lock exception or something like this.
     
  25. Offline

    tips48

    If you get a exception, what do you think? ;)
     
    wwsean08 likes this.
  26. Offline

    AbdulAbdalla

    Just because in the wiki it says "tasks which block, i.e. network read tasks" should not be executed in the main thread. And a mysql link is a kind of network connection.
     
  27. Offline

    tips48

    Which wiki? And thats a general guideline, this is an exception....(Just guessing)
     
  28. Offline

    AbdulAbdalla

    This one:
    http://wiki.bukkit.org/Scheduler_Programming

    The exception is raised, if i access one data object from two threads at the same time. I know i have to sync them, but i dont know if i have to use the "main" thread for accessing.
     
  29. Offline

    Sagacious_Zed Bukkit Docs

  30. Offline

    AbdulAbdalla

    My knowledge about threads is perfect :p I started developing complex multi thread applications for windows systems a long time ago. Its just the way java handles this stuff. Thank you anyways ... maybe this tutorial will help me. I never really learned java.
     
Thread Status:
Not open for further replies.

Share This Page