Compact Machines

Compact Machines

65M Downloads

Compact machines NPE crash with AE2 capability adapters

Keridos opened this issue ยท 10 comments

commented

Compact Machines version: 3.0.18-b281 & 278, too
Forge version: 1.12.2-14.23.5.2847
I am not using Optifine: yes
Link to Crashlog: here

Description of the problem including expected versus actual behavior:
Server crashes at start
NPE seems to be caused by the machinestate variable being null (which is odd since it is initialized)
Did some tests on this locally and could not find a proper fix for that. Adding null checks created other bugs.

Steps to reproduce:
A bit complex but I think it is possible by getting machine number 0 and set it aside. Setup ae2 system in machine number 1. Then use capability adapters from here to connect the number 0 machine via a tunnel to the ae network in machine 1. On my server I have several compact machines connected via those adapters and it worked fine. Until the day I used machine 0.

After some testing I found that I could not properly reproduce this issue on my end in a clean environment. It might be related to the fact that the world I play in has some more compact machines (17) total that are filled with machinery. I suspect that maybe there is some kind of desync where the function from the Capability adapter fires before your machinestate is properly loaded but that is just a wild guess by me.

commented

Hi, it seems that ruifung has fixed this on their end. However, I would like to make sure this is fixed on our side. In version 3.1.0 (still in testing phase), this is possibly fixed because of the modified way that the world data relating to compact machines is handled. The world data is only loaded if it is called, at which point it is loaded in a synchronized method. However, I have still seen issues with this loading method in other places in the code base. I will double check if there can be any possible issues when loading tunnel data. Please check #439 for a compiled jar (it is a few versions old but might still contain the fix). I'd like to know whether this is fixed using 3.1.0 on both server AND client, also preferably not using the fix made by ruifung just to confirm it is fixed on our side. I can work on double checking the code + getting a newer jar for you to test on. Thanks!

Correction: WorldSavedDataMachines attempts to load itself on world load of the machines dimensions (using WorldEvent.Load) at the highest subscription level. However, if the instance is retrieved before the world load event when blocks have already begun processing in another world or even the world itself because of race conditions (and other cases), it will load then if it hasn't already. This works for most cases. Looking at the code, this should fix the issues with the tunnel NPE assuming that the instance can be properly loaded at all. Because of the use of synchronized, any other point in the code that needs the instance should be stalling anyways. It is also important to note that the block ticks should be happening on the main server thread, meaning technically the synchronized keyword shouldn't be necessary at all, but it is a good fallback measure. The only thing to do now would be to build the jar and provide it to you for testing purposes.

commented

I am gonna test that jar with the old version of the ME Adapter mod to see if it crashes, too. Side note: Actually it was me who fixed the issue and ruifung only merged it. ;-)

/edit: I should be able to compile the jar myself if only the source is available ๐Ÿ‘

commented

I am gonna test that jar with the old version of the ME Adapter mod to see if it crashes, too. Side note: Actually it was me who fixed the issue and ruifung only merged it. ;-)

/edit: I should be able to compile the jar myself if only the source is available ๐Ÿ‘

Ah, got it! Sorry about that. ๐Ÿ˜… Yes, if you clone 1.12.x you should be able to build it just fine.

commented

Crashed. But with a different stacktrace: https://gist.github.com/Keridos/678bde0814c2f3aae865ab68313ee172
That was 1.12.x branch compiled. Stacktrace looks as if it actually is both the tunnel poking the Capability Adapter and vice versa.

commented

Apologies for the delayed response. The line in the stack trace seems to indicate that the world server for the machine dimension has not yet loaded. If you think about it, it makes quite a bit of sense since the world server is probably loaded after the world load event. We already knew that the ME capability block was activating before the world data was loaded on an older version of the mod, causing the original crash you experienced. The main change the beta branch brings to this specific bug is loading the world data on demand if it is needed before the world itself is loaded. This just exposed more problems with the way worldservers are retrieved. I'm make a PR to change world server retrieval to be more defensive. Another solution might simply be to wait on loading world save data for machines unless it is explicitly called by the world load event. However, this would require stalling threads or the possibility of returning null from the getInstance method, which is not ideal. I'll probably be making a PR soon, and then I'll give you the branch (and possibly jar myself) to test with.

Thanks again for your help on this!!

commented
commented

The new Version from #453 does not crash the server anymore.

commented

Great! And this is still with the older version, correct?

commented

Yes, I only changed the CM jar

/edit: Though I did not join the server, gonna need to do that to verify things are working otherwise :D

commented

Ok, thank you for your help! I'm glad that we were able to fix a bug on our side. This should make other random crashes on world load with tunnels less prevalent after the release of 3.1.0 with other mods/modpacks as well.