Async task should never access any API in Bukkit.

Discussion in 'Plugin Development' started by Doodlemeat, May 26, 2014.

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


    I need to get a confirm on this because it really confuses me.
    I am making a logger in MC just for learning and I am using an asynced task to perform better on the main thread.

    The problem however, lies in what data I can use in the asynced thread. Bukkits documentation tells me that I "should never access any API in Bukkit". I am guessing that it would be stupid to pass in a Player object? But how about bukkit types, like Location or Enums(like Material.GRASS). I do not see why I can't use those in the new thread?
    Or should I try to only use the built int types + my own?
  2. Offline


    Surely you can use enums and most another bukkit classes, but you shouldn't change world or teleport players asynchronously.
  3. Offline


    i do not understand why you ask before actually trying???

    try teleporting player or altering world in async

    try creating world in async

    just go and try, if everything works properly - it may be used in async, if exception is thrown - you shoud rework your code.
  4. Offline


    Types are things like Material.GRASS etc, you can use those, they shouldn't cause an issue. Some methods on the other hand aren't safe to use on async threads (I think there is only 1 method that is thread safe).
    definition of a method:

    You may run into concurrency issues and crash the server if you use methods from the bukkit API. Sometimes it can take hours before you run into any of these issues.

    Your best bet is to use a repeating synchronized task and break your problem up into smaller parts that wont lag the main thread.
  5. Offline


    Doodlemeat If I'm honest, I am not 100% sure why either, but I can have a guess. As RawCode said, just go and mess about with it. I think it is something to do with altering details which are sued by a variety of other objects such as world data as you end up with 2 different versions of it? (I'm not sure if that's correct or even makes sense, but it kinda does in my head)
  6. Offline


    In most cases, trial and error is the best way to learn this stuff, but with thread-safety issues it's really no use. Something can work "just fine" 1000's of times in an async task and then once in a while it just might decide to blow up, its completely unpredictable, and almost always rare.
  7. Offline


    RawCode I could go ahead and do what I think, but it's really better to ask anyone who has dealt with this before.
    You don't go ahead and build a toaster/spaceship/bukkit plugin before you know what you are dealing with.

    TopTobster5 Empire92 I think it has to be about pointers/copying objects. I'm programming in C++ usually and there I have to deal with pointers a lot, but in Java I don't think there are pointers so one may end up with as you said a copy of another world or and object when you are intended to use that same object.
  8. Offline


    Empire92 There's definitely more than one thread safe method and I wonder what this 'one method' is in your mind.
  9. Offline


    Trust me. Building a Bukkit plugin is a much more forgiving process than building a spaceship. Your analogy was a bit weird.
  10. Offline


    Rocoty Sure it is. That is why I brought a toaster :)

    Seriously though, I believe that one must know things before going into detail in multi-threaded programs. If you have experience in that field(asynced tasks) in Bukkit, please give me some advice about it..
  11. Offline


    Doodlemeat So there used to be a short list of "thread-safe" methods on the scheduler programming page:
    World.getBlockTypeIdAt(int x, int y, int z)

    But... that was removed a while back so who knows...

    AdamQpzm As for my magical one method, I guess I lied :p
    AdamQpzm likes this.
  12. Offline


    Doodlemeat I would say mostly what has been said here before. Some methods should be stayed well away from in asynchronous environments, while some are perfectly fine to use. Using a type is always fine, but don't mess it up by having multiple threads using the same object concurrently, without synchronizing properly.

    But hence my previous post, that Plugin Development is forgiving, please try for yourself. This is an excellent way of learning and is recommended by so many people on these forums. I know RawCode can't stress it enough. Trial and failure is good.

    Further, I would just like to point you to a very helpful tutorial about concurrency in Java, here. Through this you will learn a lot about multi-threading.

    Happy learning, and have fun.
  13. Offline


    your comparison is bullcrap, unlike building something "real" making plugin cost no resources Doodlemeat

    again "not going to waste time do it for me" thread, iam out permanently.
  14. Offline


    Rocoty I think your link to the tutorial is broken.
  15. Offline


    TopTobster5 likes this.
  16. Offline


    You say in your signature that you're writing a game in c++ but you don't understand the issues with concurrently modifying and accessing values? Look up on the Internet why modifying values without locking the values is a bad idea, and to everyone who said trial and error, no. You can't use trial and error for this, threading issues are significantly more complicated than that. metalhedd definitly has the right idea on this one. Doodlemeat that clause is there for a reason, you'll get yourself into all sorts of trouble if you use thread unsafe methods and values across multiple threads.
  17. Offline


    RawCode are you mad?
    mythbusterma but there is never stated anywhere WHY you should not use bukkit api. at least I could not find it.
    I have never dealt with threads in C++ so yea, it is a difference.

    So the main reason is because One can accidently modify a locked resource?
  18. Offline


    Doodlemeat Don't mind RawCode. While I do agree with most of his points, the way he's conveying them is just unnecessary and should be ignored.
  19. Offline


    In this case RawCode 's argument is flat-out wrong. You shouldn't ever "just try something" to see if it's threadsafe. you will almost always come to the same conclusion: "Yep, it works for me". and then it'll blow up for someone else.
    1mpre55, glen3b, Rocoty and 2 others like this.
  20. Offline

    Wolvereness Bukkit Team Member

    Not accessing the API asynchronously is because of the Java Memory Model and out-of-order execution. Things may work but segfault servers randomly. Things may work, but suddenly half the world ceases to exist. Things may work, but suddenly the server gets caught in an infinite loop. Things may work, but suddenly the server is deadlocked.

    Knowing what I know, and problems I've seen, things like this infuriate me:
    So, RawCode, I think you have a blatant disregard for the well-being of the users of your software. There is a very real cost with writing bad software when someone else is using it. But, of course, things like heartbleed are just fine and acceptable?

    To give something that Doodlemeat can actually take away from this thread:

    Things like material.isEdible() are perfectly fine to call from a second thread. The reason the 'no API is safe' is because if you are using a Location, then the .getBlock() method is DEFINITELY not safe to call from a second thread. Nor should you ever read/write the data in a single Location object from multiple threads (we don't have synchronization on it). You could call l.setX(), l.setY(), l.setZ(), and a second thread would only see the Y change.

    TL;DR: Everything in your second thread needs to be self-contained.
    1mpre55, nlthijs48, glen3b and 7 others like this.
  21. Offline


    On the one hand the attitude to test some things is not necessarily a bad one, because without experiments things like Minecraft/SMP might tend not to exist in the first place.

    Concerning asynchronous access of API, i would rather opt for the strict way to go by specification or by code-inspection - it hurts too, because some methods that are thread-safe are not flagged so (maybe for good reasons though, but it can mean performance penalties sending a lot of messages in the main thread). If something is to fail in unpleasant ways, it is accessing not-thread-safe API from another thread within a slightly more complex system like a CraftBukkit server with plugins.

    Argh, got to check this out, hehe.
  22. Offline


    The reason why is because most of Bukkit is not thread safe. If you don't understand why that's a problem, google "thread safety." Its an extremely complicated and not worth retyping what google will tell you, but basically boils down to occasionally everything will go wrong and data can become seriously corrupt, possibly crashing the server.

    TL; DR its a really really really bad idea

    Can we please sticky this some where? Thank you wolverness and I agree rawcodes answer was infuriating. This is great though.

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
    Last edited by a moderator: Jun 8, 2016
  23. Offline


    Poor advice. Async problems don't manifest predicatably, and it's quite likely that the async calls you make on a quiet dev server will appear fine, but cause exceptions (or even world corruption) when run on a busy production server.
    1mpre55, glen3b and NathanWolf like this.
  24. Offline


    Hello again! Thank's for all your recommendations.
    I have now tried to do an asynchronus task (a consumer) which will fetch an element from a BlockingQueue list and then process it so that it could be put into a mysql database. This works perfectly fine, but when I'm reloading the server, the console gives me this error:
    I believe it's because I didn't shut down the asynced task properly(as the image said).
    Here is my main class.
    Am I doing something wrong here? Tell me if you want the whole code, which is run on Bukkit 1.7.9 Snapshot 1
    EDIT: I can only find this thread which has the similar problem.
  25. Offline


    Doodlemeat that 'error' itself just asks you to cancel your async tasks in onDisable
  26. Offline


  27. Offline


  28. Offline


    Cancelling just removes the task from scheduling. not sure if Bukkit calls any xyzThread.interrupt methods - you might have to do that on your own.

    You may "cancel" it, but the thread might continue to exist, e.g. because you don't wait for it to finish and/or it does not respect any InterruptedException neither/nor checks if it has been interrupted.

    To get around this your task needs to check for if it has been interrupted (and/or depending on what it does catch the InterruptedException and abort then), so it can be cancelled at all. From then on i am not entirely sure if Bukkit will interrupt the thread on cancelling it, could be the BukkitTask infrastructure provides something useful (check their API docs!). Worst case is that you have to register all your tasks in the plugin, so they can be interrupted somehow (setting an "abort-flag" might suffice then, unless you need to interrupt some lengthy internet-communication or so, then you would need to store the Thread the task is running in, i assume).
  29. Offline


    I think this might be useful. This is my thread class.

    As you can see on row 33. I am using the take() method to grab an element from my LinkedBlockingQueue list.
    If I comment this out, I am not receiving the "No Author Given" problem.

    asofold How would one check if a task has been interrupted?
  30. Offline


    You can test if a thread is interrupted, for the current thread with Thread.currentThread().isInterrupted(), which can only work if called by your task during run. I am not sure if Bukkit provides somthing with BukkitTask, one would have to look that up in the API docs.

    If you store your own tasks somewhere you could also add a method to the task to interrupt it.

    It might also happen that some action throw an InterruptedException, so you should not ignore those.

    No author given?
Thread Status:
Not open for further replies.

Share This Page