What is 'Thread-Safety' and how it affects me?

People are constantly saying like 'Using the XXX part of Bukkit outside of the Main class is not thread-safe' or 'this method is not thread-safe' but honestly I don't even know what thread-safe is. When they say that I kind of just ignore it and pretend I listened. I never get issues from it. (That I know for sure of)



I have been thinking that it causes my server to crash. About every 3 reloads when I'm doing plugin development, the server crashes due to "java.lang.OutOfMemoryError: PermGen". I did some reading and that's due to memory leaks in ClassLoaders (usually) .


Could I be causing memory leaks when I don't do thread-safe methods?

How can I make my plugins thread-safe?


I'm sure this will benefit a lot of others as well.

 

I'll give you one quick example of thread safety (or lack of) to help you get a better idea of why some things can't be done in other threads. A concurrent modification exception is thrown when a collection is being modified by two different threads at the same time, or when it is being modified as it is being iterated through.

So, think of your world as a collection (which it is, a collection of blocks). If you have the main thread, where people are digging and building and stuff, then you start another thread in which you try to break blocks and build things, that's basically a concurrent modification exception.

 

Hmm.... I'm not sure if I understand


So if I have 2 events using Bukkit.broadcastMessage() At the same time, it could throw a CME ?


What about event handlers? D;

 

Lets say 2 threads read file F, with 2 inside it. Each thread has to increase value of number in file. First thread runs first and reads the file, second thread pauses it to run a bit too and reads a file as well. Now it's first thread's time again, so it increases the number and writes it into file. First thread ended, so second thread takes it's time to increase the number as well and writes it into file. But because second thread had read the file before it was actually modified by first one, both threads saw value 2 and both treads actually wrote value 3 into the file.
I'm not sure if this is brightest example of how multiple threads can mess with things, but that's how I kind of understand it. (I may be even mistaking it with processes)

 

I'm fairly certain that's a thread safe method. That's not quite what I was getting at - You're not modifying anything when you broadcast the method. If you were to hook into worldedit and paste a schematic, however, that'd be CME.

 

Ferrybig said using Bukkit.broadcastMessage() outside of JavaPlugin class wasn't thread safe.

 

http://wiki.bukkit.org/Scheduler_Programming#Thread_Safe_API_Methods

Maybe so. I was just giving one example of what would make something non threadsafe.

 

'

Is this a cause of my PermGen errors? I just contacted my hosts because this is annoying and I've never had this problem in private development (standalone double-click .bat )

I just realized, what if I used Bukkit.getServer().doWhatever() instead of Bukkit.doWhatever()
?

That sleeps the thread and I'm able to use it without problems. Guess I gotta switch it all over D_D

I think that's what he meant

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

It may also be caused by improperly clearing static variables on disable.

 

That is probably my problem. I was told to make static stuff null onDisable to use less memory.

I do like:

someClass.someList = null;


Is that bad? How should I do it?

 

That's it, but I think it's usually with bigger objects that it becomes an issue (and in total it's not THAT big of an issue anyway..)

 

Is there another way to set it to null onDisable() ? Or is that it? I'm confused if you were saying 'thats it' to me doing it wrong or me doing it right 0_o

 

That's how you null an object.

 

If you null an object Java's garbage collection would clean it up (to be exact: if there are no more references to that object, aka all references are null)

 

Until the object is defined again and loaded ?

 

Yes

 

0. at work
1. nullify stuff
2. garbage collector coming through freeing memory
3. things loading again
4. at work

If there is some stuff left unnullified left, garbage collector won't touch it. It will then pile up and eventually fill whole memory.

 

Keep in mind that, to fill the whole memory, you'd need thousands static objects, really

 

Yes, "eventually".

 

In this thread: Nobody knows what threads are.

In Minecraft, nearly all actions occur on the "main thread." What does this mean? There is a single thread on which these actions occur, meaning they all occur sequentially not simultaneously.
When a player punches a block, the interact event and the block damage event have to fire. FIRST one event fires, and fires on all of the plugins that registered the event, one at a time, according to priority. Two plugins will never get the event at the same time, because all event processing is happening on one thread. NEXT, the second event fires (I think block damage is second?), and again goes one-at-a-time through the plugins.
The same thing happens internally. The game updates what each entity wants to do (AI) one at a time, iterating through the entity list and calling the methods and waiting for them to finish before calling the next.

Nearly all events in Bukkit, and all commands, are fired synchronous to the main thread. This means that you can call any methods within Bukkit while these events are firing (with rare exceptions such as you can't kick people in the join event, but that's not a thread issue). However, the async prelogin event and the async chat event run asynchronous to the main thread. This means that they are running on a different thread. If, from one of these async events, you call a bukkit method that isn't thread-safe you will run into thread safety issues.

Thread safe? What?

Imagine a printer. You tell the printer "Print out 100 pages of this document" and give it 100 sheets of paper, then it checks for 100 pages, one-by-one grabs a sheet of paper, and prints the ink to it. If you tell the printer to print 100 pages, and then just before it prints the 98th a co-worker removes a sheet, it's going to be confused. That's because it was executing your "thread" of printing 100 pages, it had counted 100 pages, and suddenly it doesn't have 100 pages because your coworker came in during the middle of execution. In the very same way, most collections (sets, maps, lists, etc) can't tolerate a second thread (such as async events) like that irritating coworker calling a method that screws up their counting in the middle of their execution. It is expensive (meaning it takes longer, slowing things down) to make collections thread-safe. There are also plenty of other methods within minecraft itself that assume values that were a certain value at the start of their running will be the same at the end, because they are designed around the concept of those values only being modified by the main thread.

Thus, pretty much every method that affects anything within minecraft or counts anything in minecraft shoudln't be touch asynchronously. It's simply not designed to do that.

A short list (not at all a complete list) of things that aren't safe to do async:
Teleport a player
Check permissions (iterates the list, meaning that if the main thread changes a permission you'll crash)
Change permissions (if another plugin is iterating the list on the main thread, boom)
Get a block (can trigger chunk generation, if the chunk doesn't exist)
Kill any entity (including players)

Another implication of events being fired on the main thread is that if you do an action that causes a long pause you are pausing the execution of the main thread and thus slowing down the server. If your plugin makes SQL queries or huge calculations that last longer than a few milliseconds (as any SQL query may do if the server is acting up, or you're communicating over a distance) you have negatively affected performance on the server. Things that involve no calls to Bukkit methods, but can rather be supplied with the data, should be handled asynchronously.

For an example, we can talk about block change logging (logblock, hawkeye, etc). The block break event is fired on the main thread (as, again, are most events). You can collect the information you need in the main thread ( player name, block X Y Z, block type that was broken, time, etc) and then pass it off to an asynchronous task that can create the SQL query and send it off. Because you're not calling any bukkit methods, you're fine!

Need to send do an action involving calling Bukkit methods after the query is complete? From your async task, schedule a synchronous task that will call those methods!

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

So you took my first example, said I didn't know what I was talking about, then ported it to a printer example and acted like you were saying something different that I was?

 

You agreed that a method might not be callable from outside JavaPlugin as a result of thread safety issues, making me think you have no idea what threads are.

 

I was responding to him saying that broadcastMessage() wasn't thread safe, after I had previously guessed that it was.

 

Can someone please answer; can this cause my memory leaks issue?

 

If I remember correctly, yes. Thread safety can cause memory leaks. But then again, like what mbaxty said, no one here knows what threads really are.

 

It's really sad to see what "programmers" know about programming.
mbaxter explained it pretty much. Also the wiki page about the scheduler explains it a bit and last but not least we have wikipedia: http://en.wikipedia.org/wiki/Thread_safety

A short outline:
- A new class is NOT a new thread. In fact different threads can share the same classes (and do, that's why they concurrent).
- You don't define in what thread a event fires, bukkit does it (as it calls your method, not the other way around!).
- Doing non thread-safe actions may work perfectly fine at (smaller) testing servers but may instantly crash bigger servers (the bigger the server, the more code executed in one tick, the higher the chance to do concurrenting actions).
- Doing non thread-safe actions may work perfectly fine 99,99% of the time but at the 0,01% the server crashes.

 

Long story short, there are two clear ways that 'multithreading' can cause memory leaks.

• Your thread never shuts down. This means you maintain a reference to your Class<?> object. Your Cass<?> object maintains a reference to the ClassLoader. The ClassLoader maintains a reference to every other class, that may each also have other ClassLoaders.
• Your thread does something 'after' your plugin leaves 'scope.' This means you reintroduced your Class<?> object to referencable pool, leaking class loader, see previous point.

 

I was fraid of doing 'plugin = null' onDisable() because I thought it would break. Would it? Can you null a class reference object without issues?

 

Explicitly setting things to 'null' in java at shutdown is generally a bad practice. A place where it would be appropriate is a situation where you are 'finished' using something, like an IO byte[] cache. By setting it to null, you can throw an IllegalStateException if you ever attempt to use it again after it's supposed to be end-of-life. By setting a static reference to null, it may be helping your one object that you dereferenced get GCed. However, if it didn't get GCed anyway, there is a much larger, destructive problem in your software.

 

This is your reference, so you are the only one using it, so you can safely null it if you don't access it after (which shouldn't be the case after onDisable())