Check if method is thread safe

Discussion in 'Plugin Development' started by dddeeefff, Nov 5, 2012.

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

    dddeeefff

    I cannot find this information anywhere. How do I check if a method is thread safe?
    The wiki docuement on scheduler API calls says the following, but offers no advice on how to check whether a method is thread safe:

    The list at the bottom of this place states that it is incomplete


    As a bonus question, if my plugin creates the instance of a bukkit object, will methods using that object be safe as long as my plugin implements thread safety?
     
  2. Offline

    Sagacious_Zed Bukkit Docs

    Generally Bukkit API method calls are not thread safe with the exception of Scheduler and Metadata API calls. Some methods might happen to be thread-safe due to their implementation, but that is not to be relied upon.
     
    fireblast709 likes this.
  3. Offline

    Canownueasy

    Either look at the code or refer to documentation. The documentation of methods should explicitly state its thread safety... unless, of course, this is Bukkit and the method has no documentation. ;)
     
    JazzaG likes this.
  4. Offline

    dddeeefff

  5. Offline

    LucasEmanuel

    Then use a synchronized task, then you dont have to worry if a method is threadsafe or not.

    If you absolutely have to use an async task, like retrieving something from a database then send the player a message, you can surround it with a synchronized block:

    Code:
    synchronized {
        player.sendMessage("Message");
    }
    That will create a lock on the player object which only allows your thread to manipulate the player object. Keep in mind that if you do thread heavy stuff inside of this block while keeping the lock on the player, the main thread could freeze. :)
     
  6. Offline

    Njol

    Sending messages to players (and to the console) is completely thread-safe as the chat system is not handled on the main thread anymore since 1.3.
     
  7. Offline

    fireblast709

    Read the javadocs properly. Messages sent by clients are async, aka thread safe. Messages send by player.chat(""); are sync, aka not thread safe. Besides that he uses player.sendMessage(""), I don't think that is part of the normal chat system, and nevertheless not thread safe
     
  8. Offline

    Njol

    async != thread safe. 'sync' and 'async' only refer to whether the main server thread or another thread executes it. While 'sync' code is generally not thread safe the opposite can't be said for 'async' code.
    CommandSender.sendMessage is thread safe to my knowledge though i can't find out why anymore, so better ignore my previous statement and don't use it outside of the main server thread =P
     
  9. Offline

    tkausl

    Thats wrong. This will only block other Threads to get the Lock to the same Object. But if the other Thread isnt synchronized with the same Lock-Object, the other Thread can do whatever he want with the Player.
     
    Comphenix likes this.
  10. Offline

    LucasEmanuel

    Ah, yes! my bad :)
     
  11. Offline

    dddeeefff

    So how do rollback plugins such as hawkeye (that obviously create and use threads) manage to send messages to players if the method is not thread safe?

    My best guess is to call a task which returns a value, which, sends data to a delayed task (using the locking technique described above). The data sent would be a player and a message, and the delayed task would see how many messages it has to send, and send them. Would this be a good idea?

    Every message seems to be helpful and constructive in me producing the above idea, so thanks very much.
     
  12. Offline

    Sagacious_Zed Bukkit Docs

    A method that is not thread safe may still appear to behave properly.
     
  13. Offline

    Comphenix

    Sending player messages is one of the few things that are thread safe. Another one is reading the list of every online player from server (but not world, curiously).

    Generally, though, you use a scheduled task to communicate with plugins across threads. Or, you can create a common API that takes thread safety into account. Nearly all of ProtocolLib is thread safe, and most of it is accomplished without expensive and dangerous locking (synchronized).

    You should definitely take a look at the java.util.concurrent package. It's simply amazing how far you can get by just exchanging an ordinary HashMap with a lock-"free" ConcurrentHashMap. Unfortunately, there's still plenty of pitfalls to fall into, and you may may need to use locks (use ReadWriteLock if you can) and primitive wrappers (AtomicLong, ect.).

    But the biggest problem is debugging. Concurrency issues are often random and far "removed" in the code, so it's not always apparent what's causing the problem.
     
    Wingzzz likes this.
  14. Offline

    Njol

    world.getPlayers() iterates over all of the world's entities while Bukkit.getOnlinePlayers() simply returns a copy of it's player list.
     
  15. Offline

    Comphenix

    Oh, no wonder then. Seems pretty silly to iterate the entity list, when they could simply have maintained a separate list of every player in a specific world. :p

    By the way, there's a pretty good reason for "sendMessage" being thread safe. Packets are not directly sent and received by the main thread - instead, that task is delegated to a dedicated reader and writer thread per player (see NetworkManager). Packets are passed between a reader/writer thread and the main thread with queues, and naturally these queues are synchronized.
     
  16. Offline

    dddeeefff

    Okay thanks. I think I am safe not to implement some of the complicated (to me at least) stuff in ProtocolLib and the java.util.concurrent package. I've been reading more about thread concurrency and reached the conclusion that I am perfectly safe to use bukkit methods, even those which are not thread-safe, on local fields which are only used in that thread. The only thing I was worried about was the sendMessage() thing, and I have been assured it is safe, so I will use it.

    Thanks for the suggestions and the replies. What I've understood I've found interesting :)

    Nevertheless, I'm surprised that it isn't documented which methods are thread safe and which aren't - if there are so few, it would be easy to mark those which are thread safe on somewhere in the documentation.
     
Thread Status:
Not open for further replies.

Share This Page