Should pushCause/popCause be wrapped in try/finally?

Plugins Plugin Development
1

As the documentation for the CauseStackManager is currently very limited (haven’t really found any explanation at all, so I’m just guessing how to use it), I’ve had some thoughts and questions about it.

I have had to use the cause stack because I needed my plugin in the cause when the InteractInventoryEvent.Open is fired while opening a custom inventory.

My question is, how well does the stack manager handle missing calls to stackManager.popCause()? If in a plugin event, I push something to the stack but never pop it, does that break things outside of the event?

Say I have this code:

@Listener
public void onExample(ExampleEvent event) {
	Sponge.getCauseStackManager().pushCause(plugin);

	// Insert code here that is probably fine but could potentially
	// throw an exception during unforeseen circumstances
	...

	Sponge.getCauseStackManager().popCause();
}

Should I wrap the call to stackManger.popCause() in a finally case like you would do with disposable patterns, like so?

@Listener
public void onExample(ExampleEvent event) {
	Sponge.getCauseStackManager().pushCause(plugin);
	try {
		// Insert code here that is probably fine but could potentially
		// throw an exception during unforeseen circumstances
		...
	} else {
		Sponge.getCauseStackManager().popCause();
	}
}

Anyone with experience have any advice or information about how I should treat the stack manager? Or any links to resources that explains the new cause system? :slight_smile:

spongevanilla-1.12.2-7.0.0-BETA-341.jar

2

You can also use the CauseStackManager.Frames to automatically pop off causes (all causes added since the frame is constructed will be popped off). The Frame is required if you want to add contextual causes. You can also manually pop off the Frame’s through the popCauseFrame, but since the Frame is auto-closeable, you can use it in the try block.

For example:

@Listener
public void onExample(ExampleEvent event) {
	try (CauseStackManager.StackFrame frame = Sponge.getCauseStackManager().pushCauseFrame()) {
		frame.pushCause(myCause);

		// Do stuff...	
	}
}

I should also note that the PluginContainer is automatically added to the stack before the event listener is executed, so there is no need to add your plugin instance again. This is also the case when executing sync tasks through the scheduler.

I would say use try/finally if you are going with one of your examples.

1 Like
3

Aah okay, I did see something about the frames but had no idea how they worked. Thank you for providing an example. I am not sure when I should be using the frames and when I should not. Is it okay to just always assume I should use them so I can avoid the try/finally?

Also, regarding the PluginContainer automatically being added. I’m not sure if that is true in my case. Consider a InteractInventoryEvent.Open event. During this event, I want to stop the player from opening a ShulkerBox and instead create my own custom inventory and show this to the player. However, this causes the InteractInventoryEvent.Open event to fire again, which causes the custom inventory to be opened again, which causes… you get it.

Here is some sample code to replicate what I’m seeing:

@Listener
public void onOpenShulkerBox(InteractInventoryEvent.Open event,
                             @First Player player,
                             @Getter("getCause") Cause cause,
                             @Getter("getContext") EventContext context) {
	logger.info("Cause: {}", cause.toString());

	// Only on blocks
	Optional<BlockSnapshot> block = context.get(EventContextKeys.BLOCK_HIT);
	if (!block.isPresent()) return;

	// Only ShulkerBoxes
	Optional<TileEntity> tileEntity = block.get().getLocation().get().getTileEntity();
	if (tileEntity.map(te -> te.getType() != TileEntityTypes.SHULKER_BOX).orElse(false)) return;

	Inventory custom = Inventory.builder()
			.of(InventoryArchetypes.CHEST)
			.build(plugin);

	custom.query(GridInventory.class)
			.query(SlotPos.of(1, 1))
			.offer(ItemStack.builder().fromBlockSnapshot(block.get()).build());

	player.openInventory(custom);
}

This results in a StackOverflowError as explained above. I log the cause the first thing I do so we can have a look at it. All iterations before the overflow all log the same cause, which does not contain my plugin, see here:

Cause[  
   Context=Context   [  
      "sponge:block_hit"      =SpongeBlockSnapshot      {  
         worldUniqueId=31e28686-0991-4e87-96ba-aff2920041a2,
         position=(-21,
         65,
         -24         ),
         blockState=minecraft:yellow_shulker_box         [  
            facing=up
         ],
         extendedState=minecraft:yellow_shulker_box         [  
            facing=up
         ]
      },
      "sponge:used_item"      =SpongeItemStackSnapshot      {  
         itemType=ItemAir         {  
            Name=null
         },
         quantity=0
      }
   ],
   Stack=   {  
      EntityPlayerMP      [  
         'VapidLinus'         /103,
         l='temp_0',
         x=-21.13,
         y=65.00,
         z=-26.69
      ]
   }
]

Changing the code to manually push my plugin cause and aborting if the cause contains my plugin seems to fix the problem:

@Listener
public void onOpenShulkerBox(InteractInventoryEvent.Open event,
                             @First Player player,
                             @Getter("getCause") Cause cause,
                             @Getter("getContext") EventContext context) {
	logger.info("Cause: {}", cause.toString());

	if (cause.contains(plugin)) {
		logger.info("Ha, no overflow! :)");
		return;
	}

	// Only on blocks
	Optional<BlockSnapshot> block = context.get(EventContextKeys.BLOCK_HIT);
	if (!block.isPresent()) return;

	// Only ShulkerBoxes
	Optional<TileEntity> tileEntity = block.get().getLocation().get().getTileEntity();
	if (tileEntity.map(te -> te.getType() != TileEntityTypes.SHULKER_BOX).orElse(false)) return;

	try (CauseStackManager.StackFrame frame = Sponge.getCauseStackManager().pushCauseFrame()) {
		frame.pushCause(plugin);

		Inventory custom = Inventory.builder()
				.of(InventoryArchetypes.CHEST)
				.build(plugin);

		custom.query(GridInventory.class)
				.query(SlotPos.of(1, 1))
				.offer(ItemStack.builder().fromBlockSnapshot(block.get()).build());

		player.openInventory(custom);
	}
}

Now my code only gets executed once because I don’t run the rest of the code if my plugin is in the cause:

[22:22:09 INFO] [ship]: Cause: Cause[Context=Context[“sponge:block_hit”=SpongeBlockSnapshot{worldUniqueId=0d60f7d6-91e0-444a-9118-df7bf3eb1fcf, position=(-11, 65, -30), blockState=minecraft:yellow_shulker_box[facing=up], extendedState=minecraft:yellow_shulker_box[facing=up]}, “sponge:used_item”=SpongeItemStackSnapshot{itemType=ItemAir{Name=null}, quantity=0}], Stack={EntityPlayerMP[‘VapidLinus’/103, l=‘temp_0’, x=-12.27, y=65.00, z=-32.21]}]
[22:22:09 INFO] [ship]: Cause: Cause[Context=Context[“sponge:block_hit”=SpongeBlockSnapshot{worldUniqueId=0d60f7d6-91e0-444a-9118-df7bf3eb1fcf, position=(-11, 65, -30), blockState=minecraft:yellow_shulker_box[facing=up], extendedState=minecraft:yellow_shulker_box[facing=up]}, “sponge:used_item”=SpongeItemStackSnapshot{itemType=ItemAir{Name=null}, quantity=0}], Stack={se.narkling.mc.ship.ShipPlugin@20a65f45, Plugin{id=ship, name=Ship, version=1.0, description=fancy things, authors=[Linus ‘Vapid’ Närkling], source=mods\ship.jar}, EntityPlayerMP[‘VapidLinus’/103, l=‘temp_0’, x=-12.27, y=65.00, z=-32.21]}]
[22:22:09 INFO] [ship]: Ha, no overflow! :slight_smile:

There probably is a better way to do it though and I’d appreciate any advice or suggestions.

4

Just a note on why you’re getting the stack overflow:

You might actually be looking for this to be @Root. The difference here is that @First Player means get the first available Player in the cause. When you open an inventory within the event, the cause stack still contains the causes that lead up to the event, and you add to it. So, when you open the inventory, you add the plugin container to the cause stack and call the Open event. @First Player matches as you still get the player in the stack. So you open a container, add another PluginContainer to the stack, fire the Open event, match the filter… you see what is going on here.

@Root Player on the other hand only checks to see if the Player is at the head of the stack, that is, the direct cause. In your case, when you then open the inventory and put the PluginContainer on the top of the stack, this filter is no longer matching the cause stack, so this event would not be fired.

The other way you could do this, (which is the way I would do this) is to avoid calling an open event within an open event by calling a sync scheduler that fires with zero delay to open your inventory. That way, the cause stack is clear of this interaction and won’t have the player in it.

2 Likes
5

This is some very good advice, thank you. I never considered the difference between @Root and @First, I’ll be sure to keep this in mind in the future! :tada:

I tried both your @Root and Task method and they both work perfectly. The only difference I noticed was that a 0-delay task seems to execute the next tick instead of instantly, which is what I recognize coming from Spigot. That’s no problem though, I just have to cancel the event before, which I probably still should’ve been doing.

In the end, I went with the task method as you recommended and it because I don’t have to consider interacting with the stack manager. Here’s my complete code in case anyone stumbles upon this and wants an example:

@Listener
public void onOpenShulkerBox(InteractInventoryEvent.Open event,
                             @First Player player,
                             @Getter("getCause") Cause cause,
                             @Getter("getContext") EventContext context) {
	logger.info("Cause: {}", cause.toString());

	// Only on blocks
	Optional<BlockSnapshot> block = context.get(EventContextKeys.BLOCK_HIT);
	if (!block.isPresent()) return;

	// Only ShulkerBoxes
	Optional<TileEntity> tileEntity = block.get().getLocation().get().getTileEntity();
	if (tileEntity.map(te -> te.getType() != TileEntityTypes.SHULKER_BOX).orElse(false)) return;

	// Don't open ShulkerBox inventory.
	// If I don't do this, the ShulkerBox flashes open
	// for a tick before the custom inventory opens.
	event.setCancelled(true);

	Task.builder()
			.name("Override ShulkerBox inventory")
			.execute(() -> {
				Inventory custom = Inventory.builder()
						.of(InventoryArchetypes.CHEST)
						.build(plugin);

				custom.query(GridInventory.class)
						.query(SlotPos.of(1, 1))
						.offer(ItemStack.builder().fromBlockSnapshot(block.get()).build());

				player.openInventory(custom);
			})
			.delayTicks(0)
			.submit(plugin);
}

EDIT:
As you’re here, mind if I ask whether my PluginContainer not automatically being added to the cause stack when opening my custom inventory is a bug or intended?
Wondering if I should be submitting an issue about that?

6

Gut feeling says it’s a bug, because your plugin is the reason the inventory opened, it should be in the cause. I’d file that on SpongeCommon.