Is this correct thread safety?

Discussion in 'Spigot Plugin Development' started by Samistine, May 18, 2015.

  1. If this isn't safe, let me know what I should change. I am under the assumption that unloadConfig and loadConfig will not be able to run at the same time with the synchronized tag, however, I might be wrong in the fact that I need to pass in an object such as "files" for it to actually prevent simultaneous access.

    http://paste.md-5.net/wasetatobo.avrasm
     
  2. Save the info in a custom object instead of having to store the whole yml, also will be more easy get the info.
     
  3. Perhaps the blocking is a bit too overkill, but it looks thread safe at a quick glance.

    Edit: is SnakeYaml thread safe?
     
  4. When loading configuration, every ConfigurationFile from bukkit API is creating new Yaml object from SnakeYaml, so that will work from multiple threads.

    But ConfigurationFile may have problems if you will be editing it from multiple threads after loading.
     
  5. This is not thread safe if anything is accessed from an outside class.
    Since you seem to be a beginner to multithreading i would suggest:
    Letting no-one access configuration files from the outside, use synchronized getters and setters for all information, not just the config file.
    The getters and setters will make sure that no-one accesses any information before the config has been loaded, and no one sets any information while the config is being saved.

    If you are wondering why files can't be accessed from the outside, imagine this situation.
    1. Main Thread -- Techcable joins
    2. Loading task -- Started config loading
    3. Main thread -- Pretzel45 runs /gender Techcable
    4. Main thread -- Tries to get techcable's config file -- throws null pointer exception because techcable's config file hasn't been loaded yet
    If you used synchronization, the main thread would just wait until techcable's config is finished loading.
    This is still more performent than loading on the main thread, because most of the time it won't wait.

    Or this could happen (slightly more complicated)
    1. Main Thread -- Techcable joins
    2. Loading task -- Started config loading
    3. Main Thread -- Pretzel 45 runs /gender Techcable
    4. Main Thread -- Gets the configuration file map
    5. Loading task -- Finishes config loading
    6. Loading task -- puts config file in the config file map
    7. Main thread -- Gets the configuration file from the ma
    8. Main thread-- Throws concurrent modification exception because the config map has been changed
    The reason you can't expose the ConfigurationFile publicly, and instead have to create getters and setters is different.
    One thread could be getting something from the file while another threads puts it in, creating an inconsistent state.

    @DarkSeraphim
    SnakeYaml is thread safe.
     
    • Winner Winner x 2