World.getBlockTypeIdAt not really threadsafe

Back in 1.7.3, I was looking into how it would be possible to split chunk saving into another thread. In the process I discovered that World.getBlockTypeIdAt was not correctly threadsafe. leaky bukkit bug report.

The core of the issue is World.getBlockTypeIdAt uses the object World.chunkLock to protected access to the internal chunkProviderServer object, however not all access to this is protected by this sync lock.

The call tree looks like;
CraftWorld.getBlockTypeIdAt -> World.getTypeId -> World.getChunkAt -> IChunkProvider.getOrCreateChunk
(note IChunkProvider is a chunkProviderServer object, but the rabbit hole to figure that out is rather deep. And bukkit can change it)

The major issue is chunk unloading which makes changes doesn't appear to respect this lock and there are locations in the code which which directly call World.chunkProviderServer.getChunkAt rather than World.getChunkAt.

A quick grep shows the following classes should use World.getChunkAt rather than World.chunkProviderServer.getChunkAt and thus bypassing the sync lock;
World.java
WorldServer.java
EntityHuman.java
MinecraftServer.java
PlayerInstance.java
ServerConfigurationManager.java

And then, there is the case it is not threadsafe at all to load a chunk in another thread.

While there are changes designed to flag when a chunk is unloading, there is still a race condition between when a chunk starts unloading and when a chunk should be loaded. And the mess of potentially loading a chunk from another thread.