bug: hooks are global and may come into effect earlier than expected, can deadlock?

[ndrewh]

We currently allow any thread to modify hooks under most circumstances (e.g. when the thread is blocked after a run_until, or inside a hook implementation). Currently, hooks are shared between all threads (we could use dynamorio's -thread-private option instead, but this comes with higher memory overhead).

Luckily for us, these accesses all occur within Python and are thus subject to the GIL. This should mean our data structures do not need an additional lock, but there is the problem that all hooks are flushed at an arbitrary point in the future i.e. when the next thread returns from a hook (or several other circumstances where we flush all hooks).

We could implement some mechanism by which hooks are committed by a thread, and thus go into effect at a predictable time (although we cannot guarantee they go into effect at the same time in a multithreaded context).

[ndrewh]

Luckily for us, these accesses all occur within Python and are thus subject to the GIL.

This assumption is actually wrong. Hooks are flushed after return from a hook, but we release the GIL before flushing. Locking here is maybe a bit tricky since we can't hold any locks around a dr_flush_region call (since all other threads, including threads which may be in Python code, MUST be able to exit the flushed region even if within a clean-call).

I haven't come up with a design that avoids the following deadlock:

• Thread A enters hook at A1
• Thread B enters hook at A2
• Thread A flushes A2
• ... now A must wait for B to exit (releases GIL so B can run)
• Thread B flushes A1
• ... now B must wait for A to exit

We could eliminate the guarantees here by using dr_delay_flush_region (guaranteeing forward progress), but I'd rather have some guarantees about when hooks are actually installed.

This pattern (A2 flushes A1, A2 flushes A1) may be uncommon in actual usage of Pyda (frequent hook re-registration is slow anyway). So maybe we just add some accounting to flush only the hooks that changed, and then I think this becomes a non-issue. We could maybe think about some detection of these cycles, but it's not trivial to detect since it doesn't have to be a perfect address match to cause a cycle (basic block match good enough).