Skip to content

Add ILHookTransaction to batch per-target IL chain rebuilds #313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
std-microblock wants to merge 2 commits into
base: reorganize
Choose a base branch
from
+407 −35
Open

Add ILHookTransaction to batch per-target IL chain rebuilds #313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ae9d99
Add ILHookTransaction to batch per-target IL chain rebuilds
std-microblock Aug 3, 2026
5dd2dd2
Merge branch 'reorganize' into ilhook-transaction
nike4613 Sep 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 93 additions & 34 deletions src/MonoMod.RuntimeDetour/DetourManager.Managed.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,49 @@ private void RemoveNoConfigDetour(SingleManagedDetourState detour, ManagedDetour
internal readonly List<ILHookEntry> noConfigIlhooks = new();

internal int ilhookVersion;

private ILHookEntry InsertILHook(SingleILHookState ilhook)
{
if (ilhook.ManagerData is not null)
throw new InvalidOperationException("Trying to add an IL hook which was already added");

var entry = new ILHookEntry(ilhook);
ilhookVersion++;
if (entry.Config is { } cfg)
{
var listNode = new DepListNode<ILHookEntry>(cfg, entry);
var graphNode = new DepGraphNode<ILHookEntry>(listNode);

ilhookGraph.Insert(graphNode);
ilhook.ManagerData = graphNode;
}
else
{
noConfigIlhooks.Add(entry);
ilhook.ManagerData = entry;
}

return entry;
}

private void RemoveInsertedILHook(SingleILHookState ilhook, ILHookEntry entry)
{
switch (Interlocked.Exchange(ref ilhook.ManagerData, null))
{
case DepGraphNode<ILHookEntry> graphNode:
ilhookGraph.Remove(graphNode);
break;
case ILHookEntry listEntry:
noConfigIlhooks.Remove(listEntry);
break;
case null:
break;
default:
throw new NotSupportedException("bad managerdata?");
}
entry.Remove();
}

public void AddILHook(SingleILHookState ilhook, bool takeLock = true)
{
ILHookEntry entry;
Expand All @@ -435,25 +478,7 @@ public void AddILHook(SingleILHookState ilhook, bool takeLock = true)
{
if (takeLock)
detourLock.Enter(ref lockTaken);
if (ilhook.ManagerData is not null)
throw new InvalidOperationException("Trying to add an IL hook which was already added");

entry = new ILHookEntry(ilhook);
ilhookVersion++;
if (entry.Config is { } cfg)
{
var listNode = new DepListNode<ILHookEntry>(cfg, entry);
var graphNode = new DepGraphNode<ILHookEntry>(listNode);

ilhookGraph.Insert(graphNode);

ilhook.ManagerData = graphNode;
}
else
{
noConfigIlhooks.Add(entry);
ilhook.ManagerData = entry;
}
entry = InsertILHook(ilhook);

try
{
Expand All @@ -463,17 +488,7 @@ public void AddILHook(SingleILHookState ilhook, bool takeLock = true)
catch
{
// the add failed, remove the node and re-update end of chain
switch (Interlocked.Exchange(ref ilhook.ManagerData, null))
{
case DepGraphNode<ILHookEntry> gn:
ilhookGraph.Remove(gn);
break;
case ILHookEntry cn:
noConfigIlhooks.Remove(cn);
break;
default:
throw new NotSupportedException("bad managerdata?");
}
RemoveInsertedILHook(ilhook, entry);
UpdateEndOfChain();
throw;
}
Expand All @@ -491,6 +506,45 @@ public void AddILHook(SingleILHookState ilhook, bool takeLock = true)
InvokeILHookEvent(DetourManager.ILHookApplied, ILHookApplied, ilhook);
}

internal void AddILHooksBatch(IReadOnlyList<SingleILHookState> ilhooks,
Func<ILContext.Manipulator, IDisposable?>? enterManipulatorGate = null)
{
if (ilhooks.Count == 0)
return;

var added = new List<(SingleILHookState Hook, ILHookEntry Entry)>(ilhooks.Count);
var lockTaken = false;
try
{
detourLock.Enter(ref lockTaken);
foreach (var ilhook in ilhooks)
added.Add((ilhook, InsertILHook(ilhook)));

try
{
PrepareEndOfChain(added[0].Hook.Factory);
UpdateEndOfChain(enterManipulatorGate);
UpdateChain(added[^1].Hook.Factory, out _);
}
catch
{
for (var index = added.Count - 1; index >= 0; index--)
RemoveInsertedILHook(added[index].Hook, added[index].Entry);
UpdateEndOfChain(enterManipulatorGate);
UpdateChain(added[0].Hook.Factory, out _);
throw;
}
}
finally
{
if (lockTaken)
detourLock.Exit(true);
}

foreach (var (hook, _) in added)
InvokeILHookEvent(DetourManager.ILHookApplied, ILHookApplied, hook);
}

public void RemoveILHook(SingleILHookState ilhook, bool takeLock = true)
{
ILHookEntry entry;
Expand Down Expand Up @@ -556,6 +610,9 @@ private void PrepareEndOfChain(IDetourFactory factory)
}

private void UpdateEndOfChain()
=> UpdateEndOfChain(null);

private void UpdateEndOfChain(Func<ILContext.Manipulator, IDisposable?>? enterManipulatorGate)
{
Helpers.Assert(SourceClone is not null);

Expand All @@ -579,13 +636,13 @@ private void UpdateEndOfChain()
var cur = ilhookGraph.ListHead;
while (cur is not null)
{
InvokeManipulator(cur.ChainNode, def);
InvokeManipulator(cur.ChainNode, def, enterManipulatorGate);
cur = cur.Next;
}

foreach (var node in noConfigIlhooks)
{
InvokeManipulator(node, def);
InvokeManipulator(node, def, enterManipulatorGate);
}

var eoc = dmd.Generate();
Expand All @@ -598,13 +655,15 @@ private void UpdateEndOfChain()
EndOfChain = eoc;
}

private static void InvokeManipulator(ILHookEntry entry, MethodDefinition def)
private static void InvokeManipulator(ILHookEntry entry, MethodDefinition def,
Func<ILContext.Manipulator, IDisposable?>? enterManipulatorGate = null)
{
//entry.LastContext?.Dispose(); // we can't safely clean up the old context until after we've updated the chain to point at the new method
entry.IsApplied = true;
var il = new ILContext(def);
entry.CurrentContext = il;
il.Invoke(entry.Manip);
using (enterManipulatorGate?.Invoke(entry.Manip))
il.Invoke(entry.Manip);
if (il.IsReadOnly)
{
il.Dispose();
Expand Down
14 changes: 13 additions & 1 deletion src/MonoMod.RuntimeDetour/ILHook.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System;
using System.Linq.Expressions;
using System.Reflection;
using System.Threading;

namespace MonoMod.RuntimeDetour
{
Expand Down Expand Up @@ -158,6 +159,13 @@ public ILHook(MethodBase source, ILContext.Manipulator manip, DetourConfig? conf

private readonly DetourManager.ManagedDetourState state;
private readonly DetourManager.SingleILHookState hook;
private int transactionPending;

internal DetourManager.ManagedDetourState ManagedState => state;
internal DetourManager.SingleILHookState HookState => hook;

internal void SetTransactionPending(bool pending)
=> Volatile.Write(ref transactionPending, pending ? 1 : 0);

/// <summary>
/// Constructs an <see cref="ILHook"/> for the provided method using the provided manipulator and <see cref="DetourConfig"/>
Expand Down Expand Up @@ -197,7 +205,7 @@ public ILHook(MethodBase method, ILContext.Manipulator manipulator, IDetourFacto
/// <summary>
/// Gets whether or not this <see cref="ILHook"/> is applied.
/// </summary>
public bool IsApplied => hook.IsApplied;
public bool IsApplied => hook.IsApplied || Volatile.Read(ref transactionPending) != 0;
/// <summary>
/// Gets the <see cref="ILHookInfo"/> for this <see cref="ILHook"/>.
/// </summary>
Expand All @@ -223,6 +231,8 @@ public void Apply()
if (IsApplied)
return;
MMDbgLog.Trace($"Applying ILHook for {Method}");
if (ILHookTransaction.TryQueue(this))
return;
state.AddILHook(hook, !lockTaken);

@nike4613 nike4613 Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we doing implicit global-state transactions, instead of an explicit object which is given the hooks to apply? This seems like it would be very easy to get unintended and possibly wrong behavior, on top of being unintuitive.

@std-microblock std-microblock Aug 4, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same reason as #313 (comment)

}
finally
Expand All @@ -246,6 +256,8 @@ public void Undo()
if (!IsApplied)
return;
MMDbgLog.Trace($"Undoing ILHook for {Method}");
if (ILHookTransaction.TryCancel(this))
return;
state.RemoveILHook(hook, !lockTaken);
}
finally
Expand Down
Loading