[mpw]

The current implementation crashes after a single page of trampolines for imp_implementationWithBlock() is exhausted, on ARM64 that is after 127 invocations of imp_implementationWithBlock().

This patch checks if trampolines are exhausted and resets the trampoline ptr to NULL, triggering a new allocation.

[davidchisnall]

Thanks. That's an embarrassing bug. The fix isn't quite correct. Trampoline sets are supposed to be kept in a list (with each node having an inline free list). Setting the head of the list to NULL when you allocate a new node will cause a memory leak (and cause imp_removeBlock, which also seems to be failing to walk up the list, to spuriously fail).

We probably need a test that sets and unsets a few thousand blocks as IMPs.

[davidchisnall]

(CI failures are unrelated, they appear to be due to the ubuntu-latest image moving forward and needing a newer cross-libstdc++)

[sdkdimon]

Trampoline sets are supposed to be kept in a list (with each node having an inline free list). Setting the head of the list to NULL when you allocate a new node will cause a memory leak

I hit the same limit while porting a library that swizzles methods with block IMPs, and opened #439 with a fix along these lines. When every set is full, the new set is linked in at the head of the list and the older sets stay in it, each with its own free list. Nothing leaks, and trampolines freed in any set get reused.

(and cause imp_removeBlock, which also seems to be failing to walk up the list, to spuriously fail)

indexForIMP() does walk the whole list through next. imp_removeBlock() only fails here because the older sets are no longer in the list. This patch also drops a set as soon as its second-to-last trampoline is handed out, so even the IMP it has just returned can't be found. With 1000 IMPs, 889 couldn't be found or removed on AArch64 (765 on x86-64).

We probably need a test that sets and unsets a few thousand blocks as IMPs.

#439 adds Test/ManyBlockImps.m. It sets and unsets 5000 block IMPs (normal and struct-return), removes every other one, checks that the freed trampolines are reused, then removes them all and does it again. It crashes on master and fails with this patch.

The tests also turned up a few smaller bugs, fixed in the same PR:

• The trampoline stored the block that was passed in, not its Block_copy(), so a stack block left a dangling pointer.
• imp_getBlock() returned the free list link for a removed IMP instead of nil.
• Removing an IMP twice corrupted the free list.
• In builds with assertions, the first_free assertion aborted at the 128th IMP on AArch64 (int -1 vs size_t).