Skip to content

Commit

Permalink
Rewrite model loading logic to handle deadlock scenario
Browse files Browse the repository at this point in the history
  • Loading branch information
embeddedt committed Aug 19, 2023
1 parent 1aca397 commit df0bcf2
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class DynamicBakedModelProvider extends RegistrySimple<ModelResourceLocat
public static IBakedModel missingModel;
public static final ModelResourceLocation MISSING_MODEL_LOCATION = new ModelResourceLocation("builtin/missing", "missing");

private final IRegistry<ResourceLocation, IModel> modelProvider;
private final DynamicModelProvider modelProvider;
private final Map<ModelResourceLocation, IBakedModel> permanentlyLoadedBakedModels = Collections.synchronizedMap(new Object2ObjectOpenHashMap<>());
private final Cache<ModelResourceLocation, Optional<IBakedModel>> loadedBakedModels =
CacheBuilder.newBuilder()
Expand Down Expand Up @@ -71,11 +71,21 @@ private static IBakedModel getMissingIfRegistered(ModelResourceLocation location
@Override
@Nullable
public IBakedModel getObject(ModelResourceLocation location) {
try {
return loadedBakedModels.get(location, () -> Optional.ofNullable(loadBakedModel(location))).orElse(getMissingIfRegistered(location));
} catch (ExecutionException e) {
throw new RuntimeException(e.getCause());
Optional<IBakedModel> opt = loadedBakedModels.getIfPresent(location);
if(opt == null) {
synchronized (this) {
opt = loadedBakedModels.getIfPresent(location);
if(opt == null) {
opt = Optional.ofNullable(loadBakedModel(location));
loadedBakedModels.put(location, opt);
}
}
}
// avoid lambda allocation
if(opt.isPresent())
return opt.get();
else
return getMissingIfRegistered(location);
}

@Nullable
Expand Down Expand Up @@ -148,16 +158,13 @@ private IBakedModel loadBakedModel(ModelResourceLocation location) {
};

private static IBakedModel bakeAndCheckTextures(ResourceLocation location, IModel model, VertexFormat format) {
// TODO log when textures missing
synchronized (DynamicBakedModelProvider.class) {
IBakedModel bakedModel = model.bake(model.getDefaultState(), format, loggingTextureGetter);
if(!MISSING_MODEL_LOCATION.equals(location)) {
DynamicModelBakeEvent event = new DynamicModelBakeEvent(location, model, bakedModel);
MinecraftForge.EVENT_BUS.post(event);
return event.bakedModel;
} else
return bakedModel;
}
IBakedModel bakedModel = model.bake(model.getDefaultState(), format, loggingTextureGetter);
if(!MISSING_MODEL_LOCATION.equals(location)) {
DynamicModelBakeEvent event = new DynamicModelBakeEvent(location, model, bakedModel);
MinecraftForge.EVENT_BUS.post(event);
return event.bakedModel;
} else
return bakedModel;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import net.minecraft.util.registry.IRegistry;
import net.minecraftforge.client.model.ICustomModelLoader;
import net.minecraftforge.client.model.IModel;
import net.minecraftforge.client.model.ModelLoader;
import net.minecraftforge.client.model.ModelLoaderRegistry;
import net.minecraftforge.fml.common.ObfuscationReflectionHelper;
import org.apache.logging.log4j.LogManager;
Expand All @@ -18,7 +17,6 @@

import javax.annotation.Nullable;
import java.util.*;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;

public class DynamicModelProvider implements IRegistry<ResourceLocation, IModel> {
Expand Down Expand Up @@ -57,11 +55,21 @@ public DynamicModelProvider(Set<ICustomModelLoader> loaders) {
@Nullable
@Override
public IModel getObject(ResourceLocation location) {
try {
return loadedModels.get(location, () -> Optional.ofNullable(loadModelFromBlockstateOrInventory(location))).orElse(null);
} catch (ExecutionException e) {
throw new RuntimeException(e.getCause());
Optional<IModel> opt = loadedModels.getIfPresent(location);
if(opt == null) {
synchronized (this) {
opt = loadedModels.getIfPresent(location);
if(opt == null) {
try {
opt = Optional.ofNullable(loadModelFromBlockstateOrInventory(location));
} catch(Exception e) {
throw new RuntimeException(e);
}
loadedModels.put(location, opt);
}
}
}
return opt.orElse(null);
}

private static final Map<ResourceLocation, IModel> MODEL_LOADER_REGISTRY_CACHE = ObfuscationReflectionHelper.getPrivateValue(ModelLoaderRegistry.class, null, "cache");
Expand Down Expand Up @@ -177,12 +185,10 @@ private IModel loadModel(ResourceLocation location, Set<ResourceLocation> loadSt
throw new ModelLoaderRegistry.LoaderException("No suitable loader found for the model " + location);
}

synchronized (DynamicModelProvider.class) {
try {
model = accepted.loadModel(actualLocation);
} catch (Exception e) {
throw new ModelLoaderRegistry.LoaderException("Exception loading model " + location + " with loader " + accepted, e);
}
try {
model = accepted.loadModel(actualLocation);
} catch (Exception e) {
throw new ModelLoaderRegistry.LoaderException("Exception loading model " + location + " with loader " + accepted, e);
}

if(model == null)
Expand Down

0 comments on commit df0bcf2

Please sign in to comment.