From 93e759794ff121f3a08967fe6612e0828fb5f0cb Mon Sep 17 00:00:00 2001 From: Rosie Val Date: Wed, 29 Jul 2026 00:13:26 +0100 Subject: [PATCH] Fix NewHelpMenu: restore [p]help, close memory leak, parallelize perm checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Loading this cog called bot.remove_command("help") with no replacement command ever defined and no restore on unload — simply loading it broke [p]help bot-wide, permanently, until process restart. Traced through Red's own source (redbot/core/commands/help.py) and confirmed the default help command already routes through bot.send_help_for(), which is exactly what this cog hooks to render CV2 output — so the fix is to just not remove the command at all; the CV2 conversion now actually works as intended too, since its hook point is reachable again. _active_views leaked one permanent dict entry (holding the whole rendered page/embed list) per help/menu invocation — every on_timeout and "close" button already called view.stop(), but nothing was popping the matching entry except one specific code path. Added a single _track_view() helper that wraps stop() for automatic eviction and applied it at all 5 insertion points instead of just the one. Parallelized the per-command can_run() permission checks in gather_bot_help_data/gather_cog_help_data (previously one await at a time for every bot/cog command on every single [p]help call). Also added the admin_or_permissions check to `cv2 version`, the one subcommand missing it. Co-Authored-By: Claude Sonnet 5 --- newhelpmenu/__init__.py | 13 ++++++++----- newhelpmenu/formatter.py | 38 ++++++++++++++++++++++++++------------ newhelpmenu/newhelpmenu.py | 37 +++++++++++++++++++++++++------------ 3 files changed, 59 insertions(+), 29 deletions(-) diff --git a/newhelpmenu/__init__.py b/newhelpmenu/__init__.py index 0b3d525..7cbff06 100644 --- a/newhelpmenu/__init__.py +++ b/newhelpmenu/__init__.py @@ -2,11 +2,14 @@ from .newhelpmenu import NewHelpMenu async def setup(bot): - # Remove Red's built-in help command BEFORE adding the cog, - # so our cog's @commands.command(name="help") doesn't conflict. - original_help = bot.remove_command("help") - + # Red's own default help command (and any cog's ctx.send_help()) already + # routes through bot.send_help_for() internally — see + # redbot/core/commands/help.py. That's exactly the method this cog hooks + # to render CV2 output, so the built-in help command must stay in place + # for that hook to ever fire. Removing it here used to just delete + # [p]help bot-wide with nothing to replace it (there was never an actual + # replacement `help` command defined), and cog_unload never restored it + # either — simplest correct fix is to not remove it at all. cog = NewHelpMenu(bot) - cog._original_help_command = original_help # store for restore on unload await bot.add_cog(cog) await cog.initialize() diff --git a/newhelpmenu/formatter.py b/newhelpmenu/formatter.py index 5442cca..fedc416 100644 --- a/newhelpmenu/formatter.py +++ b/newhelpmenu/formatter.py @@ -73,14 +73,20 @@ async def gather_bot_help_data( pending_checks.append((cmd, cog_name)) - # Run permission checks (this is the slow part) - for cmd, cog_name in pending_checks: + # Run permission checks concurrently — each is an independent await (often + # a Config lookup via Red's permission system), and with a few hundred + # bot commands doing them one at a time made every single [p]help call + # pay for that latency serially. + async def _safe_can_run(cmd: commands.Command) -> bool: try: - if await cmd.can_run(ctx): - cog_commands[cog_name].append(cmd) + return await cmd.can_run(ctx) except Exception: - # Permission check failed, skip this command - pass + return False + + allowed_flags = await asyncio.gather(*(_safe_can_run(cmd) for cmd, _ in pending_checks)) + for (cmd, cog_name), allowed in zip(pending_checks, allowed_flags): + if allowed: + cog_commands[cog_name].append(cmd) cog_to_category: Dict[str, str] = {} for cat_name, cog_list in categories.items(): @@ -124,14 +130,22 @@ async def gather_cog_help_data( if not cog_doc: cog_doc = cog.help or cog.__doc__ or "No description." - cmds = [] - for cmd in sorted(cog.get_commands(), key=lambda c: c.qualified_name): - if cmd.hidden and not show_hidden: - continue + candidates = [ + c for c in sorted(cog.get_commands(), key=lambda c: c.qualified_name) + if not (c.hidden and not show_hidden) + ] + + async def _safe_can_run(cmd: commands.Command) -> bool: try: - if not await cmd.can_run(ctx): - continue + return await cmd.can_run(ctx) except Exception: + return False + + allowed_flags = await asyncio.gather(*(_safe_can_run(c) for c in candidates)) + + cmds = [] + for cmd, allowed in zip(candidates, allowed_flags): + if not allowed: continue sig = cmd.signature or "" short = cmd.format_shortdoc_for_context(ctx) if hasattr(cmd, "format_shortdoc_for_context") else (cmd.short_doc or "No description") diff --git a/newhelpmenu/newhelpmenu.py b/newhelpmenu/newhelpmenu.py index 18f3360..18c9950 100644 --- a/newhelpmenu/newhelpmenu.py +++ b/newhelpmenu/newhelpmenu.py @@ -104,6 +104,24 @@ class NewHelpMenu(commands.Cog): self._settings_cache: Dict[int, Tuple[Dict, float]] = {} self._cache_ttl = 60.0 # Cache for 60 seconds + def _track_view(self, msg_id: int, view: "ui.LayoutView") -> None: + """Register a view in _active_views with automatic eviction on stop. + + Every view stored here previously stayed forever: on_timeout and every + "close" button already call view.stop(), but nothing popped the + matching entry, so every single help/menu send permanently retained + one dict entry (holding the whole rendered page/embed list) — an + unbounded memory leak on any active server. + """ + self._active_views[msg_id] = view + original_stop = view.stop + + def cleanup_stop(): + original_stop() + self._active_views.pop(msg_id, None) + + view.stop = cleanup_stop + async def cog_load(self) -> None: """Called when cog is loaded — set up monkey patches and replace help.""" await self._apply_patches() @@ -286,13 +304,7 @@ class NewHelpMenu(commands.Cog): # Track for cleanup with automatic removal on stop if msg: - cog_ref._active_views[msg.id] = layout - # Add cleanup callback - original_stop = layout.stop - def cleanup_stop(): - original_stop() - cog_ref._active_views.pop(msg.id, None) - layout.stop = cleanup_stop + cog_ref._track_view(msg.id, layout) return msg @@ -373,7 +385,7 @@ class NewHelpMenu(commands.Cog): ) msg = await destination.send(view=view) view.message = msg - self._active_views[msg.id] = view + self._track_view(msg.id, view) else: embeds, select_opts = build_bot_help_embeds( ctx, self.bot, cat_data, total, accent_color=accent @@ -399,7 +411,7 @@ class NewHelpMenu(commands.Cog): for comp in components: layout.add_item(comp) msg = await destination.send(view=layout) - self._active_views[msg.id] = layout + self._track_view(msg.id, layout) else: embed = build_command_help_embed(ctx, data, accent_color=accent) await destination.send(embed=embed) @@ -414,7 +426,7 @@ class NewHelpMenu(commands.Cog): for comp in components: layout.add_item(comp) msg = await destination.send(view=layout) - self._active_views[msg.id] = layout + self._track_view(msg.id, layout) else: embed = build_cog_help_embed(ctx, cog_name, cog_doc, cmds, accent_color=accent) await destination.send(embed=embed) @@ -445,7 +457,7 @@ class NewHelpMenu(commands.Cog): layout = ui.LayoutView() layout.add_item(container) msg = await destination.send(view=layout) - self._active_views[msg.id] = layout + self._track_view(msg.id, layout) else: bot_avatar = self.bot.user.display_avatar.url if self.bot.user else None embed = discord.Embed(color=discord.Colour(accent)) @@ -1024,6 +1036,7 @@ class NewHelpMenu(commands.Cog): await ctx.send("✅ All Components V2 settings reset to defaults (disabled).") @cv2.command(name="version") + @checks.admin_or_permissions(manage_guild=True) async def cv2_version(self, ctx: commands.Context): """Show the cog version.""" await ctx.send(f"**NewHelpMenu** v{self.__version__} by {self.__author__}") @@ -1073,5 +1086,5 @@ class NewHelpMenu(commands.Cog): ) msg = await ctx.send(view=view) view.message = msg - self._active_views[msg.id] = view + self._track_view(msg.id, view) return msg