Resolved How to prevent rapid button clicks?

[alikim]

A few events are raised rapidly from the same thread where the handler is, I want this handler to process only one event (doesn't matter which one).
Since it's all in one thread, I can't use lock or other technique for multi-thread sync, so I simply use a bool flag, like so:

    bool busy = false;
    static internal readonly EventHandler ClickHandler = (object? sender, EventArgs e) =>
    {
        if (busy) return;
        busy = true;

        //...
    };

It seems though that this handler can be called while the first call is still checking and setting the flag, is there a safer solution?

 

[Skydiver]

The WinForms UI is single threaded. You cannot have something interrupt you between line 4 (the check for busy) and line 5 (setting busy).

I suspect your problem is more complex than what you are showing above. Perhaps you are actually experiencing a re-entrancy rather than a concurrency issue. Perhaps you are getting on multiple threads due to use of a background worker, or calling something that uses a thread pool.

If you can post a minimal WinForms program that demonstrates the issue, that would be very helpful.

 

[KOZ60]

Does it be that clicked message is queued and event occurs multiple times?
Can this be resolved?

private static async void AsyncButtonClick(object? sender, EventArgs e) {
    var button = (Button?)sender;
    button!.Enabled = false;
    try {
        await Task.Run(() => {
            // your code
        });
    } finally {
        button!.Enabled = true;
    }
}

or

private static void SyncButtonClick(object? sender, EventArgs e) {
    var button = (Button?)sender;
    button!.Enabled = false;
    try {

        // your code

    } finally {
        Application.DoEvents();
        button!.Enabled = true;
    }
}

 

[alikim]

My apologies for the confusion, I'm not experiencing this issue, I'm looking for the most reliable way to prevent it.
I have a few AI agents playing a game, they all run in their own threads, and, when they calculate a move, they all raise an event on the UI thread, I'm trying to make sure that ClickHandler for that event is run only once. It's not a matter of why and how the game is designed this way, it's the matter of how to handle this situation.

Here is what I'm trying to understand: the invocations are inserted into the main message loop on UI thread, is that correct? In case there are many of such calls, will they be queued somehow? So that the first call can execute and lock itself with a bool flag and other calls will bounce? Is it 100% reliable approach?

As to the invocation code, I use Raise to invoke events on the same thread and InvokeFromMainThread to invoke on the UI thread, like so:

// event manager class EM

...
static event EventHandler<Point> EvtAIMoved = delegate { };
static event EventHandler<Game.Roster> EvtAIMakeMove = delegate { };
static event EventHandler EvtGameTie = delegate { };
...


internal enum Evt
{
    ...
    AIMakeMove,
    AIMoved,
    GameTie,
    ...
}

static readonly Dictionary<Evt, Delegate> dict = new() {
    ...
    { Evt.AIMakeMove, EvtAIMakeMove },
    { Evt.AIMoved, EvtAIMoved },
    { Evt.GameTie, EvtGameTie },
    ...
};

static internal void Raise<E>(Evt enm, object sender, E e)
{
    if (!dict.TryGetValue(enm, out var _evt))
        throw new NotImplementedException($"EM.Raise : no event for Evt.{enm}");

    bool nonGeneric = _evt.GetType() == typeof(EventHandler);

    if (nonGeneric)
    {
        var evtNG = (EventHandler)_evt;
        evtNG?.Invoke(sender, new EventArgs());

    } else
    {
        var evtG = (EventHandler<E>)_evt;
        evtG?.Invoke(sender, e);
    }
}

static internal AppForm? uiThread;
static internal void InvokeFromMainThread(Action lambda) => uiThread?.Invoke(lambda);


// AI class

...
EM.InvokeFromMainThread(() => EM.Raise(EM.Evt.AIMoved, new { }, new Point(tile.row, tile.col)));
...


// UI thread

static bool busy;
...
static internal readonly EventHandler<Point> ClickHandler = (object? sender, Point e) =>
    {
            if (busy) return;
            busy = true;

            if (sender is not IComponent iComp)
                throw new Exception($"TurnWheel.ClickHandler: '{sender}' is not IComponent");

            if (CurPlayerIsHuman) DisableUICb();

            iComp.IsLocked = true;

            Game.Update(CurPlayer, new Tile(e.X, e.Y));
    };
...

 

[KOZ60]

If AppForm is a class that inherits from System.Windows.Forms.Form, locking is not necessary.

 

[Skydiver]

I have a few AI agents playing a game, they all run in their own threads, and, when they calculate a move, they all raise an event on the UI thread,

And there lies the first issue. You shouldn't be sending an event into the UI thread. The AI agents should be sending messages into your own game event queue. Your UI should also be sending events into your game event queue. Your game loop should be taking events from the queue one at a time and processing them.

 

[Skydiver]

Here is what I'm trying to understand: the invocations are inserted into the main message loop on UI thread, is that correct? In case there are many of such calls, will they be queued somehow? So that the first call can execute and lock itself with a bool flag and other calls will bounce? Is it 100% reliable approach?

Messages that are sent using PostMessgae() are not simply inserted into the message loop of the UI thread, they are appended to the end of the UI thread message queue. When that message is dispatched, the handler is executed. Most messages sent using SendMessage() look like they are getting inserted into the message queue, but what is actually happening is more like a co-function call which will come back to the point when SendMessage() was called. So in other words it's just like another function call which will execute the handler. In either case, nothing will interrupt the currently executing handler, unless you call another handler (via SendMessage()). So like in your original post, nothing else will execute between your line 4 and 5 within the same thread.

 

[Skydiver]

private static void SyncButtonClick(object? sender, EventArgs e) {
    var button = (Button?)sender;
    button!.Enabled = false;
    try {

        // your code

    } finally {
        Application.DoEvents();
        button!.Enabled = true;
    }
}

Beware of using Application.DoEvents(). You are forcing the message pump to run and can set yourself up for some nasty re-entrancy bugs that are hard to reproduce and diagnose. It's a terrible hack that a lot of VB6 programmers use to make their single threaded apps look more responsive, but can be dangerous when used without appropriate safeguards to prevent re-entrancy.

 

[alikim]

And there lies the first issue. You shouldn't be sending an event into the UI thread. The AI agents should be sending messages into your own game event queue. Your UI should also be sending events into your game event queue. Your game loop should be taking events from the queue one at a time and processing them.

This is my first attempt ever to make a mini game engine (tic-tac-toe ) I'm pretty sure there are better thought thru architectures.