RFTools: RFTools Builder block leaks chunkload tickets

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.

About this issue

Original URL
State: closed
Created 7 years ago
Reactions: 1
Comments: 15

Most upvoted comments

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.

positivereturn on Feb 1, 2018

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.

DarkMorford on Oct 18, 2017

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.

DarkMorford on Oct 17, 2017

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.

DarkMorford on Oct 16, 2017