RFTools Builder block leaks chunkload tickets
DarkMorford opened this issue ยท 15 comments
During an investigation into some chunkloading issues on my server, I found that RFTools had 29 tickets listed in the forcedchunks.dat
file for the overworld. As we don't currently have that many quarries running - certainly not in the overworld, since we have a mining dimension - I decided a little more digging was in order.
In proxy/CommonProxy.java
I see that you register a ForgeChunkManager.LoadingCallback
with an empty ticketsLoaded
method. However, the documentation for ForgeChunkManager
states that "tickets unwanted by the mod must be disposed of manually unless the mod is an OrderedLoadingCallback
instance". Since your LoadingCallback
does not call ForgeChunkManager#releaseTicket
, I believe that there is a resource leak. These ignored but unreleased tickets will continue to pile up until the per-mod limit in forgeChunkLoading.cfg
is reached (which, by default, is 200 tickets), and then the Builder will no longer be able to request new tickets from the chunk manager.
I see two potential solutions for this issue, but perhaps you can think of another. The first is simply to iterate over the offered tickets in ticketsLoaded
and call releaseTicket
on each one. This would indicate to the chunk manager that RFTools does not care about the ticket and also return the ticket to the pool so that it is available for the next Builder. The other solution is to implement the OrderedLoadingCallback
interface and return an empty List
from its ticketsLoaded
method. This tells the chunk manager that none of the offered tickets are of interest, and that none of them should be subsequently offered to the ticketsLoaded
method of LoadingCallback
.
@McJty Where is it handled? I can patch it via Mixin.
I am seeing this exact issue on my server currently. At server startup, the following error is showing the log:
[03:52:35] [Server thread/INFO] [FML]: The mod rftools has attempted to allocate a chunkloading ticket beyond it's currently allocated maximum: 200
Builder's are unable to operate, showing this error:
Based on DarkMorford's description, this exactly matches the behavior he indicated.
Hmm I'm a bit confused about this. When am I supposed to release this tickets? You tell me how but not when
In an ideal case, the ticket would be released as soon as it is no longer needed (i.e., when the Quarry has finished its operation or the block is broken). This has the advantage of keeping as many tickets as possible available for future operations, but may require a fair amount of code in the Builder's TileEntity.
A simpler alternative would be to release all the tickets when they are offered to the mod at server startup. Something like:
ForgeChunkManager.setForcedChunkLoadingCallback(RFTools.instance, new ForgeChunkManager.LoadingCallback() {
@Override
public void ticketsLoaded(List<ForgeChunkManager.Ticket> tickets, World world) {
for (ForgeChunkManager.Ticket ticket : tickets) {
ForgeChunkManager.releaseTicket(ticket);
}
}
});
This approach requires very little in terms of code changes, but has the disadvantage of still keeping tickets allocated when they are no longer necessary. On a very busy server that is not restarted regularly, it's possible that the mod's allotment of tickets could be held up by inactive quarries, and a new operation would be unable to request one until after the server was restarted.
Releasing tickets when the server starts? What's the point of that? When the server starts why would there be any tickets to release in the first place? Sorry but this totally confuses me...
In the Builder I have:
@Override
public void invalidate() {
super.invalidate();
chunkUnload();
}
private void chunkUnload() {
if (forcedChunk != null && ticket != null) {
ForgeChunkManager.unforceChunk(ticket, forcedChunk);
forcedChunk = null;
}
}
Wouldn't it be better to do the releaseTicket() there then? I already unload the chunk at that point
When a mod successfully obtains a chunkloading ticket with requestTicket
or changes the ticket's NBT with Ticket#getModData
, ForgeChunkManager
records that allocation in the dimension's forcedchunks.dat
file. This file persists ticket information even after the server is shut down. When a dimension is loaded, ForgeChunkManager
reads that world's forcedchunks.dat
and calls each mod's registered LoadingCallback
, passing it a list of the tickets it had stored. At this point it's up to the individual mod to decide what to do with each ticket. If the ticket is ignored (i.e., no action is taken), then no chunks are loaded, but the ticket remains allocated and will be offered to the mod's LoadingCallback
again the next time the world is loaded. The mod may, of course, use the ticket in a call to forceChunk
(perhaps using the stored NBT data to determine which chunk(s) to load), or it could call releaseTicket
, declaring that the offered ticket is no longer useful and freeing it for a future allocation. I hope this clears up the confusion for you; feel free to ask if there's something you're still not clear on.
I'm not familiar with all the different cases in which TileEntity#invalidate
might be called, but it seems like a reasonable place to release the ticket. Whether at load time or when the TE is invalidated, the key point is that tickets that are no longer needed have to be explicitly released, otherwise the allocations will build up in forcedchunks.dat
until the per-mod limit is reached.
So you see that requested tickets are persisted even when the world unloads? Are you sure of that? I never heard of that before
Quite sure. I'm actually working on my own chunkloader mod, and I've observed the behavior I've described here in my development process.
My chunkloader block requests a ticket from ForgeChunkManager
when it's placed in the world. If I then stop the server, I can look in the forcedchunks.dat
file with an NBT editor and see the ticket there, along with the metadata I added recording the block's position. When the server starts up again, that ticket gets passed to my playerTicketsLoaded
method and then to ticketsLoaded
, where I use it to force-load the chunks surrounding the saved block. If I break the block (which unforces the chunks and releases the ticket), it no longer shows up in forcedchunks.dat
.
2020 and builder still does not clean up after it's self. I am playing the MC.eternal using rftools-1.12-7.73.jar and with one builder(quarry clear card) the forcedchunks.dat listed over 200, causing the builder to stop working (chunk not available.). Deleting the forcedchunks.dat seems to have fixed it for now.
Is there any update on this issue? We have the same problem on our Network. RFTools is just not deleting these tickets. Our server only had around 6TPS with these tickets. Deleting the forcedchunks.dat
resulted that in a 10TPS increase, with over 17 players.