MightyPirates / TIS-3D

TIS-100 inspired low-tech computing in Minecraft.
https://www.curseforge.com/minecraft/mc-mods/tis-3d
Other
108 stars 35 forks source link

Serial Port Module deadlocks read operations that are performed while it's value changes #76

Closed CD4017BE closed 6 years ago

CD4017BE commented 6 years ago

The bit Splitter from my mod AutomatedRedstone provides a Serial Interface to allow communication between both mods which would otherwise be very annoying if you could only use the 4-bit bandwidth of redstone modules. This device should not be responsible for any signal blocking since both canRead() and canWrite() just return true;

When any module reads a value from a serial port that is attached to that device and its Serial Interface happens to change the value returned in peek() from one tick to the next during that read process then that read operation never completes and deadlocks the module.

From what I've seen in the Tis-3D source code, a transmission from one module to another takes multiple ticks and if the provided value changes while a transmission is in progress then the transmission resets to use the fresher value instead. Usually this would only delay the transmission by one cycle, but in case of the Serial Port for what ever reason it blocks that process completely until you reset the controller.

This only happens in 1.12 (tested with TIS-3D-MC1.12-1.3.0.15) but NOT in 1.11 (tested with TIS-3D-MC1.11.2-1.3.0.53)

original issue: CD4017BE/AutomatedRedstone#5

fnuecke commented 6 years ago

Yay for super slow response times (sorry about that). I'll try to reproduce this in the next couple of days (and try to remember how the serial interface stuff worked...)

fnuecke commented 6 years ago

Queue black screen

Text appears: "Later"

Black screen fades

Sooooo. Uhm. Sorry about that. Tried to build the dev version of your mod, but didn't get very far; would you mind sending me the JARs? From looking at the code I don't really see where something could lock up. Also just to be clear, you're saying the TIS-3D computer locks down, or just the module (?), not the game, right?

esotericist commented 6 years ago

As the person who originally submitted the issue to @CD4017BE in the other repo I can say with confidence the game does not lock up.

Beyond that, I am very unclear as to what exactly is happening. The distinction between a deadlock in the TIS-3D machine, in a TIS-3D module, or in the thing the TIS-3D module is interfacing with is very difficult to discern from the outside when unfamiliar with all of the code involved.

Edit: although to be honest I haven't worked with that other mod in quite some time, so I don't know if the problem is still reproducable.

fnuecke commented 6 years ago

One way to test is have random module next to an audio module somewhere on the same TIS-3D computer away from the serial port module (i.e. on a casing connected to the same controller).

I'm afraid I haven't used AutomatedRedstone at all -- I'm not really doing a lot with MC anymore in general :/ - so a rough breakdown of the setup used to reproduce this would be great. I assumed the module was only in the dev branch and not published yet, but if I can just grab a JAR from Curse, point me at the right one please, thanks!

esotericist commented 6 years ago

I'm afraid I can't be much more use. This was back in November, so I genuinely can't remember any more information than is in the original issue in the Automated Redstone repo. If sufficient information isn't in that issue or the video linked in the issue, I'd have to attempt to recreate the problem from scratch as well, and I don't have an equivalent test environment anymore, and I am not presently using Automated Redstone. :(

I can say for sure at the time I was working with release jars of all relevant mods.

CD4017BE commented 6 years ago

The newest 1.12 version of AutomatedRedstone is this one. The required version of my library is linked under dependencies. The -deobf.jar should (for both mods) theoretically work in dev-environment mods folder.

A quite reliable setup for reproducing the deadlock is: TIS-3D casing with Execution Module on one face and Serial Interface Module on an adjacent face. And a Bit Splitter Wire place next to it. The Bit Splitter needs to have the side facing the Serial Interface configured to output at 16-bit bandwidth or so and has another side connected to a Redstone Potentiometer placed next to it.

Then let the Execution Module just read values from Serial Interface using something like MOV LEFT ACC. And rapidly click at different locations on the Redstone Potentiometer to change it's value. It's important that the Redstone value changes fast enough that it happens while the read instruction is still in progress.

As far as I had seen, I think that only the execution module deadlocks in it's read instruction but not the entire structure/controller.

fnuecke commented 6 years ago

Thanks a lot for the write-up! Will try to repro with that tomorrow.

fnuecke commented 6 years ago

Well, nevermind tomorrow. Found it. It was indeed basically a race condition, when a transfer was canceled (in this case due to an external value changing) in the same tick as the operation completed but the transferring pipe's update method wasn't called yet the pipe just broke. Fixed now!

Thanks again so much to both of you for helping me track this one down, and thanks for your patience!