3

Would anyone care to explain to me how the value of this.oBalance.QouteBalance is evaluated to be true for being less than zero when it clearly isn't? Please see image below.

Am I missing something fundamental when it comes to comparing doubles in C#??

enter image description here

public double QouteBalance { get; set; }

UpdateBalance_PositionOpenned() is not being called in a loop, but is being called as part of a more complex event driven procedure that runs on the ticks of a timer (order of milliseconds)

EDIT: Pardon the code if it's messy but I couldn't edit it as this was a run-time error after quite a long run-time so was afraid wouldn't be able to recreate it. The Exception message is not correct and just a reminder for myself. The code after the exception is code I forgot to comment out before starting this particular run.

EDIT 2: I am building and running in Release Mode.

EDIT 3: Pardon my ignorance, but it would seem that I am in fact running in a multi-threaded environment since this code is being called as part of a more complex object method that gets executed on the ticks (Events) of a timer. Would it possible to ask the timer to wait until all code inside its event handler has finished before it can tick again?

EDIT 4: Since this has been established to be a multi-threading issue; I will try to give wider context to arrive at an optimized solution.

I have a Timer object, which executes the following on every tick:

  1. Run a background worker to read data from file
  2. When background worker finishes reading data from file, raise an Event
  3. In the event handler, run object code that calls the method below (in the image) and other multiple routines, including GUI updates.

I suppose this problem can be avoided by using the timer Tick events to read the from file but changing this will break other parts of my code.

Improve this question
26
  • 6
    Multi-threading? Commented Mar 4, 2014 at 20:15
  • 1
    If you step over the current line, is the exception actually thrown? If not, your symbols are probably out of date, try rebuilding. Commented Mar 4, 2014 at 20:16
  • 3
    Note: It's a little odd to try and execute a line of code after throwing an exception. With my current week. I'd never trust the Visual Studio Debugger to give an accurate value. Try outputting it somewhere. PS - you don't show us the definition of QouteBalance anywhere that I'm seeing. Certainly 0 is not a double in this case... Commented Mar 4, 2014 at 20:16
  • 2
    @ArmenSafieh-Garabedian: If you are representing money, please use decimal instead of double: stackoverflow.com/questions/693372/… Commented Mar 4, 2014 at 20:31
  • 3
    Given your latest update, you lied to us! This is multithreaded. Events are fired on different threads. It's probably a simple race condition since you're accessing shared variables from multiple threads. Commented Mar 4, 2014 at 20:45

3 Answers 3

Reset to default
2

You're accessing shared variables from multiple threads. It's probably a race condition where one thread has thrown the error but by the time the debugger has caught and attached, the variable's value has changed.

You would need to look at implementing synchronizing logic like locking around the shared variables, etc.

Edit: To answer your edit:

You can't really tell the timer to not tick (well you can, but then you're starting and stopping and even after calling Stop you might still receive a few more events depending on how fast they are being dispatched). That said, you could look at Interlocked namespace and use it to set and clear and IsBusy flag. If your tick method fires and sees you're already working, it just sits out that round and waits for a future tick to handle work. I wouldn't say it's a great paradigm but it's an option.

The reason I specify using the Interlocked class versus just using a shared variable against comes down to the fact you're access from multiple threads at once. If you're not using Interlocked, you could get two ticks both checking the value and getting an answer they can proceed before they've flipped the flag to keep others out. You'd hit the same problem.

The more traditional way of synchronizing access to shared data member is with locking but you'll quickly run into problems with the tick events firing too quickly and they'll start to back up on you.

Edit 2: To answer your question about an approach to synchronizing the data with shared variables on multiple threads, it really depends on what you're doing specifically. We have a very small window into what your application is doing so I'm going to piece this together from all the comments and answers in hopes it will inform your design choice.

What follows is pseudo-code. This is based on a question you asked which suggests you don't need to do work on every tick. The tick itself isn't important, it just needs to keep coming in. Based on that premise, we can use a flagging system to check if you're busy.

...
Timer.Start(Handle_Tick)
...

public void Handle_Tick(...)
{
    //Check to see if we're already busy. We don't need to "pump" the work if
    //we're already processing.
    if (IsBusy)
        return;

    try
    {
        IsBusy = true;

        //Perform your work
    }
    finally
    {
        IsBusy = false;
    }
}

In this case, IsBusy could be a volatile bool, it could be accessed with Interlocked namespace methods, it could be a locking, etc. What you choose is up to you.

If this premise is incorrect and you do in fact have to do work with every tick of the timer, this won't work for you. You're throwing away ticks that come in when you're busy. You'd need to implement a synchronized queue if you wanted to keep hold of every tick that came in. If your frequency is high, you'll have to be careful as you'll eventually overflow.

Improve this answer
Sign up to request clarification or add additional context in comments.

18 Comments

Edited to respond to your latest edit. You can't tell timer to skip ticks but you can use a shared flag to indicate you want to ignore a tick or two while you catch up. It's not great code but it's an option.
Would you say that I am more than likely not to have this issue if my ticks were slower than the time it takes to execute all the code in the event handler?
@ArmenSafieh-Garabedian It would have to be "quite a lot slower". Even then, it would be nothing even remotely close to a guarantee because of the way most GUI processes (especially GC'ed ones) tend to "hiccup".
You would be less likely to hit them but they can still happen. Events don't always fire immediately on tick. There are a number of things that could cause your event handler to be delayed in its execution. If one tick gets delayed and another comes in, you'd have the same problem. Now if it takes a couple millseconds to execute your code and the ticks are 1 minute apart, it's such a low likelihood you'd probably never need to worry. But I need to stress this is still not good! It depends on how much your data integrity matters.
@ErikNoren I think I'd be tempted to use Monitor.TryEnter() with a very very short wait timeout. If I didn't get the lock, I'd just exit the highest level event (from timer) at that point. (Further, once I got the lock, I could check it'd been long enough since last start/stop times that I should do something that won't bog the system) If TryEnter(foo, 1) was too slow, then I'd consider a controlling thread of my own instead of a timer perhaps...
|
1

This isn't really an answer but:

UpdateBalance_PositionOpenned() is not being called in a loop, but is being called as part of a more complex event driven procedure that runs on the ticks of a timer (order of milliseconds)

see:

Multi-threading? – abatishchev 30 mins ago

Tight timer driven event-loop on the order of milliseconds probably has all the problems of threads, and will be almost entirely impossible to trouble-shoot with a step-through debugger. Stuff is happening way faster than you can hit 'F10'. Not to mention, you're accessing a variable from a different thread each event cycle, but there's no synchronization in sight.

Improve this answer

3 Comments

Bingo!!! timer on order of milliseconds from threadpool means quite likely more than one thread is running through the code,and all the math there is unprotected from synchronization issues. That's it!!!
@LB2 Blame abatishchev he got it first! Debug tracing (rather than step-through debugging) will often help here, especially if you use synchronization to slow things down as needed...
@ArmenSafieh-Garabedian +1 to you for learning something important!
0

Not really a full answer but too much for a comment
This is how I could code defensively
Local scope leads to less unexpected stuff
And it make code easier to debug and test 

public void updateBalance(double amount, double fee, out double balance)
{
    try
    {
        balance = amount * (1.0 + fee);
        if (balance < 0.0) balance = 0.0;
    }
    catch (Exception Ex)
    {
        System.Diagnostics.Debug.WriteLine(Ex.Message);
        throw Ex;
    }
}

Value type is copied so even if then input variable for amount changed while the method was executing the value for amount in the method would not.

Now the out balance without locks is a different story.

Improve this answer

2 Comments

So if the tick event happened before this method finished executing I'd still have a problem right?
It depends on how you handle balance outside the method. But at least the method has protected itself. If you are in a multithreaded environment it is problematic to refer to variables outside the method. There is an overhead as the value is copied in the method call.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.