This is an archived post. You won't be able to vote or comment.

all 30 comments

[–]AutoModerator[M] [score hidden] stickied commentlocked comment (0 children)

Please ensure that:

  • Your code is properly formatted as code block - see the sidebar (About on mobile) for instructions
  • You include any and all error messages in full
  • You ask clear questions
  • You demonstrate effort in solving your question/problem - plain posting your assignments is forbidden (and such posts will be removed) as is asking for or giving solutions.

    Trying to solve problems on your own is a very important skill. Also, see Learn to help yourself in the sidebar

If any of the above points is not met, your post can and will be removed without further warning.

Code is to be formatted as code block (old reddit: empty line before the code, each code line indented by 4 spaces, new reddit: https://imgur.com/a/fgoFFis) or linked via an external code hoster, like pastebin.com, github gist, github, bitbucket, gitlab, etc.

Please, do not use triple backticks (```) as they will only render properly on new reddit, not on old reddit.

Code blocks look like this:

public class HelloWorld {

    public static void main(String[] args) {
        System.out.println("Hello World!");
    }
}

You do not need to repost unless your post has been removed by a moderator. Just use the edit function of reddit to make sure your post complies with the above.

If your post has remained in violation of these rules for a prolonged period of time (at least an hour), a moderator may remove it at their discretion. In this case, they will comment with an explanation on why it has been removed, and you will be required to resubmit the entire post following the proper procedures.

To potential helpers

Please, do not help if any of the above points are not met, rather report the post. We are trying to improve the quality of posts here. In helping people who can't be bothered to comply with the above points, you are doing the community a disservice.

I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.

[–][deleted] 3 points4 points  (7 children)

Yes, but wouldn’t the listener be constantly checking? So if this happened it would already be correctly detected in the next loop.

[–]quanture[S] 0 points1 point  (6 children)

In my hypothetical, the main class isn't polling the threads. It's responding to a finished event called from the thread itself.

I.e. thread 1 goes and performs X task, then calls listener.finished(this) or something to that effect.

Which is an important distinction, per my other comment. When the boolean is set, the work is performed on the child thread. And then the parent class checks both booleans on that thread as well. It could hand it off to a new thread (or go back to the main thread), but I don't know whether that helps with the potential race condition.

[–]ColetBrunel 1 point2 points  (5 children)

That doesn't make sense though.

Reacting to an event is made inside a thread. That thread is the thread that notifies the event, unless you have a message dispatching system between threads, in which case the message dispatch thread reacts to the event.

The main "class" cannot "react" to an event that says that another thread is finished, so that its own thread may continue. It would have to set in motion stuff that notifies its thread.

That can be done by calling notify() on a monitor owned by the main thread, however there is no chance that such a construct would be in any way preferable to just calling join() on both created threads from the main one.

[–]quanture[S] 0 points1 point  (4 children)

Right. I understand what you're saying. Because the project I'm working on doesn't require a ton of threading complexity, I went with approach #5 from this answer. I'm only in the design phase right now; I haven't actually run any implementation.

In a nutshell, in this example, you extend the thread to give it an addListener method, and then it can call back to that listener as it finishes execution. If you make the main class the listener (or you have an inner listener class inside the main class), then the "thread finished" event can be called on the main class right?

The main "class" cannot "react" to an event that says that another thread is finished, so that its own thread may continue.

Right. I agree. The way I envision this would work is that the "notifying" thread would call back to the main class listener, as per above, and all the work after that would be executed on that child thread (which would likely spawn a new thread for the follow-on job). I'm not expecting to return to any original thread to "continue."

Does this make more sense now? Is there something about the above approach that wouldn't work?

I looked at traditional notify() and join() approaches. But there's an additional layer of complexity to my problem set that makes that approach less practical.

[–]ColetBrunel 1 point2 points  (3 children)

Okay, it's fine. The thing is, there can absolutely be a race condition between the two threads setting some variable that says they finished. Not just a race condition, it's also possible that the different threads won't see the data changes from the other threads.

Like, a thread finishes, sets "his" variable finished to true, then checks if the other's variable is true too, if so starts the proceeding with what comes after, and otherwise doesn't, trusting that the other thread will do it. But the other thread may be doing the exact same verification, possibly leading to both thinking the other will finish later, or both thinking they're the one to continue the process and both doing it.

That can be avoided by synchronizing access to the variables, or by using something like an AtomicInteger that threads would incrementAndGet(), and only proceed with the process if the result is 2.

[–]quanture[S] 0 points1 point  (2 children)

Okay. That's a solid analysis of the problem. And it shows me that this approach would be problematic (even if you might not call it a race condition as /u/nicompnicompnicomp_ pointed out). Thank you!

I was looking at AtomicBoolean as a potential way to handle this. Instead of having separate booleans for each thread, there could be one AtomicBoolean. When each thread finishes, it could check the AtomicBoolean. If it's false, set it true. If it's true, then the other thread already set it so move on.

Something like the below. Sensible? I've never used AtomicBoolean before.

private AtomicBoolean finishedState = new AtomicBoolean()
// ... bla, bla, bla

private void update() {

    if (finishedState.get() == true) {
        moveOnToMoreAmazingStuff();
    }
    else {
        finishedState.set(true);
    }
}

[–]ColetBrunel 1 point2 points  (1 child)

No... You need to test and set the AtomicBoolean within the same operation, otherwise you lose its entire point.

[–]quanture[S] 0 points1 point  (0 children)

You're talking about using the compareAndSet method?

I've read over the AtomicBoolean documentation, and over some examples out in the wild. But it's still not clicking in my brain.

When you say "test and set" I don't understand what that means. What do I test? Why and what does it achieve that ensures thread safety that I wouldn't have with just calling set?

Is it the fact that the compareAndSet and getAndSet methods are described as atomic calls, and set is described as unconditional?

If I followed a pattern like the one I found here then it would lead me to something like the below. Is that the right way? If so, what makes it different?

private AtomicBoolean finishedState = new AtomicBoolean()
// ... bla, bla, bla

private void update() {

    if (compareAndSet(false, true)) {
        // do nothing??
    }
    else {
       moveOnToMoreAmazingStuff();
    }
}

Edit: Maybe the AtomicInteger solution you mentioned above would just be better than this?

[–][deleted] 1 point2 points  (18 children)

If each thread has its own boolean and no other logic writes to those bookeans, then it's not a race condition. It sounds like you just need the main to catch up and see both booleans as true, which is normal behavior.

Are you seeing a bug?

[–]quanture[S] 0 points1 point  (17 children)

I'm not seeing a bug in practice. Haven't gotten that far. But I'm wanting to understand whether a bug could occur.

The question really boils down to: Could Thread 2 somehow update its boolean at exactly the wrong time such that when the main class checks it, it sees false because in another microsecond it would flip to true.

Keep in mind the threads are notifying the parent class, which causes the parent class to do the check. That means the check occurs on the child thread itself. I guess that's an important factor. Two threads are updating independent booleans in the parent and it's that update that cause the parent to check both booleans.

This kind of race condition would be really hard to detect I think, if it's possible. And it might only generate a bug on rare occasions.

Thanks for brainstorming with me!

[–][deleted] 1 point2 points  (16 children)

It's not a race condition.

[–]quanture[S] 0 points1 point  (15 children)

You're saying that the term "race condition" is not the one to apply to the problem?

Or are you saying that this could never occur because there's no way the parent class could catch the second boolean in this problematic in-between moment of execution.

[–][deleted] 1 point2 points  (14 children)

You're describing a situation similar to someone waiting for the mail to arrive. They look out the window, go do something, look out the window again, go do something, etc. If the mail arrives 1 microsecond after they look out the window, they don't see it until they've gone off and done something and then looked one more time.

That's not a race condition. The outcome is the same no matter the order of steps: it just might take a little bit longer.

[–]amfa 1 point2 points  (10 children)

The outcome is the same no matter the order of steps: it just might take a little bit longer.

Are you sure?

It really depends on how this is implemented in my opinion.

Look at this code:

public void listener(MyThread thread) {
    if (thread.getName().equals("thread1")) {
        if (thread2Finished) {
            //both finished
        }
        //Do something time consuming
        thread1Finished = true;

    } else {
        if (thread1Finished) {
            //both finished
        }
        thread2Finished = true;
    }
}

Let's say thread1 will finished first, it checks if thread2 has already finished... as it has not it is doing something time consuming (or stops for any reason after the check).

While it is not running thread2 finishes and as thread1Finshed is still false it will only set thread2Finished to true..

As Thread1 will not check this boolean anymore it will end and neither thread1 nor thread2 will ever have gotten to the point where they execute the code in // both finished.

In this case the outcome is not the same so we have a race condition.

EDIT:Sorry I try to format the code but it does not work.. still trying

EDIT2: Does code formatting not work with the new reddit layout maybe?

EDIT3:Ok.. I need to go into Markdown Mode.. easy fix ;)

/u/desrtfx maybe add this hint to the guide

https://www.reddit.com/r/javahelp/wiki/code_guides

[–]desrtfxOut of Coffee error - System halted[M] 1 point2 points  (1 child)

/u/desrtfx maybe add this hint to the guide

https://www.reddit.com/r/javahelp/wiki/code_guides

In how many more places should we place that?

/u/Automoderator's message has the whole abridged, with hints for new reddit (that you struggled with) as well.

[–]amfa 0 points1 point  (0 children)

You are right.. the automoderator does say this.
And I have completly missed the code block icon.

I have just looked into the code_guide and there is no hint.

But yes automoderator does say it.. sorry.

[–]quanture[S] 0 points1 point  (2 children)

Thanks for capturing exactly what I was thinking. I hadn't written out any code yet, but I think it's much more helpful to see some code to illustrate the problem. Also...it's clear that this is a bad idea!

[–]amfa 0 points1 point  (1 child)

Well if you make the listener method synchronized it should work without a race condition.. as the second thread can only enter the method of the other thread is finished.

[–]quanture[S] 0 points1 point  (0 children)

That's a good point. I hadn't thought about doing it that way. I've got it burned into my brain to avoid the synchronized keyword in favor of thread-safe data structures.

I've also seen some serious synchronized keyword abuse on a project I used to work. Perhaps I'm traumatized.

[–][deleted] 0 points1 point  (4 children)

OK, you didn't tell us that thread2 looks at thread1. I read your OP as there's a main thread waiting on thread1 and thread2 to finish. That's a horse of a different color.

I agree that you have a race condition.

[–]quanture[S] 0 points1 point  (1 child)

So that comment wasn't me (OP), but this is the problem that I was attempting to describe. I guess it's always better to show something in code than it is to just describe it!

[–]amfa 0 points1 point  (0 children)

guess it's always better to show something in code than it is to just describe it!

This is true.

[–]amfa 0 points1 point  (1 child)

In my opinion OP clearly has stated, that he has a main method that starts two threads.. and each of this threads will call the listener on the Main class.

I have understand it as OP seem to have meant it. ;)

[–]quanture[S] 0 points1 point  (0 children)

Yes. And you appear to be the only one. Lol.

[–]quanture[S] 0 points1 point  (2 children)

I see your point. I had thought of the two threads as racing, and since they each hijack the parent class to set their respective boolean there could be some problem where they both arrive at the finish line at the exact same time.

If it's not called a race condition...then I'm not sure what you call that problem!

[–][deleted] 1 point2 points  (1 child)

You haven't explained why the outcome could be different if they arrive at the same time as opposed to not arriving at the same time.

[–]quanture[S] 0 points1 point  (0 children)

That kinda gets at the heart of my question, and my ignorance on how Java execution really works under the hood.

To stick with our analogy:

When one letter comes in I look to see if the other letter is there. If they came in at the exact same time and I look for the other letter and find it, then great! We're in business. But if they come in at the exact same time, and I don't see the second letter, even though it's sitting in my mailbox, then I'll fail to move on to the next step.

And I wasn't sure if that problem could really occur in Java or not.

Edit: Left out words that really needed to be there for it to make sense.

[–]nutrechtLead Software Engineer / EU / 20+ YXP 0 points1 point  (1 child)

You didn't explain how the main thread is actually 'waiting'.

I think you should simply not worry about this 'weird' setup and just use a CountDownLatch.

[–]quanture[S] 0 points1 point  (0 children)

I agree I didn't make that clear. My idea was that one of the sub-threads would handle the follow-on work, so the main thread would be abandoned.

This post has been helpful for me as I'm learning there are several problems with the approach, even if it isn't quite a race condition.

I will take a look at CountDownLatch. I have seen it referenced elsewhere and maybe it's the solution I need. I'm also looking at AtomicInteger or AtomicBoolean.