0

I've read about VB6's threading model, and found this link very helpful.

With the following points in mind...

Do VB6 event handlers run in separate threads?

Not really, because there aren't separate threads. Your code runs on a single thread, wrapped in the service-like architecture I described above. Most of what you talk to that is threaded is other COM objects which have their own apartments. So to communicate back and forth, you are basically doing RPC calls when the threads talk to each other: you aren't directly manipulating them.

Among other things, the VB6 program had a timer that woke up every 4 seconds, manipulated some global variables and went back to sleep, while the main program was doing its thing. I can't understand why this didn't result in collisions.

The "timer" is on a separate thread created for the timer, but when it calls into your code, you are guaranteed not to interrupt any other functions, because the function calls are basically queued one at a time in the thread.

... I've attempted to implement VB6's event handling behavior in the code below.

ActionManager.cs

public class ActionManager : IDisposable
{
    private readonly BlockingCollection<Action> ActionQueue = new BlockingCollection<Action>(new ConcurrentQueue<Action>());

    public ActionManager()
    {
        
    }

    public void Kickoff()
    {
        // Start consumer thread
        new Thread(ExecuteLoop)
        {
            IsBackground = true
        }.Start();
    }

    public void AddAction(Action action)
    {
        ActionQueue.Add(action);
    }

    private void ExecuteLoop()
    {
        // Blocks until new actions are available
        foreach (var action in ActionQueue.GetConsumingEnumerable())
        {
            action.Invoke();
        }
    }

    public void Dispose()
    {
        ActionQueue.CompleteAdding();
        ActionQueue.Dispose();
    }
}

MainForm.cs

public partial class MainForm : Form
{
    public ActionManager actionManager = new ActionManager();
    
    public MainForm()
    {
        InitializeComponent();
    }
    
    private void MainForm_Load()
    {
        // Perform preparatory steps, such as initializing resources,
        // configuring settings, etc.
        // (Insert preparatory steps here)
        
        // Once preparatory steps are complete, start the ActionManager
        actionManager.Kickoff();
    }
    
    // Event handler for when the Timer's specified interval has elapsed
    private void Timer_Tick(object sender, EventArgs e)
    {
        actionManager.AddAction(() => {
            // (Insert timer event steps here)
        });
    }
    
    // Event handler for when SomeButton is clicked
    private void SomeButton_Click(object sender, EventArgs e)
    {
        actionManager.AddAction(() => {
            // (Insert button click event steps here)
        });
    }
}

An ActionManager manages an event queue by executing each event one after the other. Any type of event, such as mouse clicks, timer ticks, network packet arrivals, and the like, will enqueue their respective event handling code to the event queue. This way, the code will run "on a single thread," which will also handle the problem of unsynchronized global variables.

Is this a correct implementation? Please share your thoughts!

Community
  • 1
  • 1
NewbGains
  • 13
  • 3
  • I can't see this working as intended. First you are creating a background thread to run on, so it will run on a thread other than the UI thread. Second you are adding actions each time the timer ticks or a button is pressed which will result in duplicate actions (they aren't removed when you execute them). I'm not sure though *why* you want to try to shoehorn into a single-threading model, "global variables" is also a VB-gone-by thing that you shouldn't try to use in C#. – Ron Beyer Apr 05 '18 at 18:27
  • You are also only executing the thread loop once when it is created and never again after that. `ExecuteLoop` needs to have a loop inside it, otherwise it will only run one time and the thread will quit. `Timer_Tick` if it is a `Windows.Forms.Timer` will run on the UI thread anyway, so no need to worry about that. – Ron Beyer Apr 05 '18 at 18:28
  • Is this so you can port a VB6 legacy app into the .NET environment, but were running into threading issues before? – Kevin Apr 05 '18 at 18:34
  • @RonBeyer is the background thread an issue if I use Control.Invoke() to avoid cross-thread errors? About the duplicate actions, I believe that GetConsumingEnumerable will remove the items - but in any case, will removing after executing solve the issue of duplicate items? I'm migrating a VB6 application into C#, which is why I'm interested in VB6's thread model. The legacy application also has plenty of global variables, which I will use static variables to represent, and will later refactor to avoid global variables. – NewbGains Apr 05 '18 at 18:40
  • @Kevin Yes! The legacy app has plenty of event handlers that access global variables that are shared between events. – NewbGains Apr 05 '18 at 18:40
  • @RonBeyer About the ExecuteLoop, I believe that CompleteAdding() is necessary to stop the loop. Question about Windows.Forms.Timer- since it runs on the UI thread, does that mean that there won't be illegal cross-thread actions in its event handler? – NewbGains Apr 05 '18 at 18:44
  • Yes, you can interact with controls in the timer tick without cross-thread issues. `Control.Invoke` will avoid cross-thread issues. I missed the `ConsumingEnumerable` which will remove the items, but duplicates may still exist (button clicked twice fast for example). The `ExecuteLoop` is never restarted, so it will run once when you `Kickoff` and never again, you either need to make a loop, or start new threads, which puts you back at the original problem. – Ron Beyer Apr 05 '18 at 18:47
  • @RonBeyer GetConsumingEnumerable blocks if there are no items in the collection until CompleteAdding has been called so there is not need for another loop. – Mike Zboray Apr 05 '18 at 18:52
  • @RonBeyer No, the OP's implementation of a message loop is correct. Entirely unnecessary in this situation, but it is a working message loop. Yes, if multiple messages are added to the queue, they will all be processed. That's the message loop doing its job. If a message is added once, it will be processed exactly once with the OP's code. The thread also will continue processing messages until the message loop is disposed of. It will not stop processing messages early unless the OP disposes of their action manager early, which they aren't doing (in any code shown anyway). – Servy Apr 05 '18 at 18:55
  • @mikez I'll retract my opinion on the loop mechanism, `BlockingCollection` wasn't the one I was thinking of when I wrote that (I was thinking `ConcurrentQueue`. I still believe trying to shoehorn the VB6 threading model into C# isn't the best idea though. – Ron Beyer Apr 05 '18 at 19:12
  • @Servy See above comment. – Ron Beyer Apr 05 '18 at 19:12
  • @RonBeyer But the threading model is identical between the two (at least in this respect; both have a message loop, both have a mechanism to queue actions to it, all UI interactions need to run in that thread, etc.). No "shoehorning" is required. – Servy Apr 05 '18 at 19:15
  • @Servy Right, but C# is a multi-threaded model whereas VB6 is a single-threaded one. Non-UI elements (events from things like Serial Port or Network) run on the same thread as UI elements, which isn't the case in C# and was the problem the OP was trying to solve. The "shoehorning" is trying to push everything onto the UI thread. – Ron Beyer Apr 05 '18 at 19:17
  • @RonBeyer thank you for the help. I'll work with C#'s multi-threaded model, and use separate threads to handle background tasks like receiving network packets. – NewbGains Apr 06 '18 at 14:34

2 Answers2

4

What you have is a somewhat decent starting place for a custom message loop, if you were to begin writing your own UI framework from scratch. But you're using winforms, you're not writing your own UI framework from scratch. Winforms already has its own message loop that processes messages, and a mechanism for scheduling work to run in that loop. You don't need to create any of that from scratch. All of the events fired from the winforms controls will already be firing in the UI thread, so you don't need to create your own special UI thread and manage scheduling actions into it.

In fact doing so would cause problems, as you would end up having the UI thread that winforms is using to manage its UI objects, and you would have your second thread that you're creating. If you ever used any UI controls in that thread things would break as they are designed to only be used from the winforms UI thread.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • Wow, TIL. So will events fired from Winforms controls execute one after the other? About things by accessing them from threads other than the UI thread, isn't it possible to use Control.Invoke() to solve that issue? Also, what if there are events that are not limited to just Winforms controls? I am porting a legacy VB6 application which fires events when network packets arrive. To port this behavior, I used a NetworkStream, which is not a Winforms control. – NewbGains Apr 05 '18 at 18:59
  • @NewbGains They're all running in the same thread (the UI thread), so they can't run at the same time. `Control.Invoke` is merely the mechanism used to schedule work to run in the existing UI message loop. You wouldn't want to have the UI thread, and your own second UI thread, and try to constantly pass messages between them, if they're both doing UI work. If you have *non*-UI work, then it can make sense to schedule a non-UI thread to do some work for you, and then schedule that work to run in the UI thread when it's done. There's no need for a second message loop to do that. – Servy Apr 05 '18 at 19:04
  • @NewbGains If you have a non-UI control that is firing events outside of the UI thread, and you need to run code in the UI thread (which won't always be the case), then you should be scheduling work to run in the UI thread. – Servy Apr 05 '18 at 19:05
  • I think I had a misunderstanding of the Winforms message loop behavior coming in... So will Winforms handle any and all events sequentially? So that way, there’s no need to schedule events manually? – NewbGains Apr 05 '18 at 19:21
  • @NewbGains It will handle all UI events in the same thread. You will need to schedule actions to run on the UI thread if you are running code in a non-UI thread that needs to interact with the UI. – Servy Apr 05 '18 at 19:29
  • Got it. Since Control.Invoke() will have code run on the UI thread anyway, correct? The main reason I implemented the message loop was my concern over events accessing shared data. Will Winforms handle events in a thread one after the other, without interrupting active events? – NewbGains Apr 05 '18 at 19:36
  • @NewbGains That depends on what you do in those events. – Servy Apr 05 '18 at 20:09
  • I see. Assuming that there is only one thread (the UI thread) running, and that all event handlers execute synchronous code, then will the Winforms message loop do exactly what my message loop is doing, except the former will be on the UI thread, and the latter will be on its own thread? – NewbGains Apr 05 '18 at 20:37
  • @NewbGains Not *exactly* the same, but morally equivalent code, yes. It has something functionally similar where there's a while loop and a queue of actions and it tries to pull in a new action and process it and then goes onto the next. There are differences in specifics, and features of the winforms message loop that you haven't added, but the overall idea is there. – Servy Apr 05 '18 at 20:45
  • I see, thank you. At the moment, I would just like to make sure that I don't run into any synchronization issues. After learning more about events (particularly that they will not interrupt another event in the same thread), I believe the next step is to modify my code by removing the ActionManager message loop, and leaving message handling to the Winforms message loop. Speaking of features of the Winforms message loop, do you have any recommended reading/resources to learn more? – NewbGains Apr 05 '18 at 21:01
  • @NewbGains I do not. – Servy Apr 05 '18 at 21:02
1

(I figured I should ask in the comments first if my suspicion about a legacy app was right.)

Okay, time for the bad news: you should NOT do this. Please, please, please, do NOT do this. I'm telling you as a developer that has been in your shoes that this will NOT end well if you try to go down this road.

Here's what's going on. You've got a legacy app - and it probably does a lot of things that are very important for the company.

But the problem is, it's likely not written very well, it's cranky, and it did not port very well into the modern .NET world.

Now, you can try to go down the road of shoehorning .NET into the VB6 model of the world... but all you've done is kick the can down the road. You've still got a badly-written, cranky legacy app that you're still having to maintain - and worse, you're having to maintain the .NET-to-VB6-threading-approach as well.

I can guarantee you that the correct approach is to Redesign/Rearchitect it. Write out what it does, ask yourself if there's anything you can do to improve the process, and write it from scratch in .NET. Several reasons:

  1. You're going to have a more stable end product
  2. You're going to spend FAR less time maintaining the new product
  3. You'd have to rearchitect the program eventually anyways.

If it helps, let me tell you a story of an old job I had. A coworker and I were both responsible for porting VB6 apps into .NET. He had a tire inspection app, and I had a rubber mixing app.

  • He tried porting his existing VB6 app into .NET, getting all the language differences worked out, GUI/Thread issues altered, etc
  • I sat down with a rep from the user area, and went ahead just rewriting the rubber mixing app.

... I was done much sooner than the coworker, my app was far more user-friendly, and it was a heck of a lot less of a maintenance issue.

Management likely will not like hearing advice that you should rewrite the whole thing. But you need to push and fight for this. If it helps, point out that most software dev time isn't on new coding, it's on maintaining existing software. It might take more time up front to get it rewritten (even that's not a given) but it'll pay for itself very quickly in the long run.

Kevin
  • 2,133
  • 1
  • 9
  • 21
  • Agree 110%. VB6 (COM) was for a different era. It had it's place 20 years ago. In the past I've been where kevin was - VB6 app (which started with VBA & Access, ugh) with numerous 3rd party controls (some of which were no longer available). The management solution was to port to c# keeping the same architecture. This was a total disaster. Problems in the old app were just brought over to the port. It was finally assigned to my group where management was finally convinced to re-write correctly (MVC, etc). This thing calculated pay and commissions. Scary how it was originally done. – JazzmanJim Apr 05 '18 at 19:46
  • Thanks for the advice Kevin and @bednarjm. I'll redesign the program in .NET. Per Servy's advice, I'll get rid of my own message loop and leave message handling to Winforms. I'll also check out MVC and allow multiple threads for background tasks. Does this sound like a good way to get started? – NewbGains Apr 06 '18 at 14:29
  • Sort of. To be honest, I'd suggest taking a step back and asking, "What does the app need to do?" Pretend you're programming it *from scratch*, and figure out what the correct technology needs to be. It'd be a shame to shoehorn a specific implementation type just because it's similar to how it operates now. You might find that it'd work better as an MVC app, or maybe a thin-client form app that talks to a WebService, or maybe something else entirely. – Kevin Apr 06 '18 at 14:59
  • Got it. Thank you for the great advice Kevin. – NewbGains Apr 09 '18 at 17:53