AutoHotkey icon indicating copy to clipboard operation
AutoHotkey copied to clipboard

CallbackCreateSync

Open DartVanya opened this issue 1 year ago • 1 comments

CallbackCreate - add '$' option to create callback that is safe to pass to multithreaded APIs. If callback thread not equal to AHK thread execution will be synchronized via SendMessage invoke.

DartVanya avatar Dec 27 '24 21:12 DartVanya

I preferred to avoid handling any window messages in a way which allows execution of arbitrary code. I touched on this in RegisterSyncCallback for v2.

Perhaps that concern is unfounded, as there are other methods that a process with sufficient permission could use to inject code into our process, and any number of window messages which could be sent with invalid pointers or malformed data. The message would be blocked by default if sent from a lower-integrity process (due to UIPI).

This does have the benefit of simplicity, which is good for code size, performance and maintainability.

I think there would be some benefit in having this be the default behaviour, as allowing the callback to proceed on the wrong thread is likely to only cause frustration. Crashes are the best thing that can happen; multiple threads trying to use the same internal global variables can have unpredictable results, and lines apparently executing out of sequence can be quite confusing. Some errors are subtle or might appear only under some conditions, so might not bring an author's attention to the need for synchronization.

I would consider backward-compatibility a non-issue because executing script on other threads was never supported, but some authors might have been utilizing it successfully, through some miracle. I think that few users understand the program internals (or multi-threading) well-enough to be a good judge of when it is safe to multi-thread script functions (unless the judgement is "it's never safe").

One reason to make synchronization "opt in" would be that future changes could make it safer to execute script on other threads (such as by reducing reliance on internal global/static variables), but given that any script function intended to be used that way would have to be carefully designed, it would probably be better to require the author to explicitly opt out of synchronization.

If we add an option, I do not think I would choose to represent it with $. It Synchronizes using SendMessage. I suppose you might have chosen $ over S because FAST contains S. The existing option parsing is very lazy, and just checks for letters in the string rather than parsing each option. CallbackCreate should perhaps be changed to validate the options as per the documentation, to detect errors and reserve other characters/words for future use.

Lexikos avatar Feb 22 '25 08:02 Lexikos