79083565

Date: 2024-10-13 16:09:40
Score: 1.5
Natty:
Report link

You do not appear to understand how java's threading model works at all. It's hard to highlight any particular line. I'll try:

This:

if (mustCallLater) {
  unstartedRunnables.add(new Socket<A>(notRunningThread, threadPostfix));
}

is fundamentally broken. You are attemping to use advanced footgun concepts such as double checked locking, and yet you also say things like:

@marcinj Can you answer as an unit-test?

Which indicates you should put the foot guns away.

JMM isn't deterministic

The fundamental problem with your understanding is in how you appear to think that the behaviour of java's threading, i.e. when a write in one thread can be seen in another thread, is deterministic. It is not - hence, it is not possible to 'show' that your code is broken with a simple unit test.

The thing you need to wrap your head around is this:

If you attempt to observe in line B a change that line A made and there is no HappensBefore(A, B) then your code is broken.

And that's because the JMM (Java Memory Model; that section of the JVM spec that describes how observability works) says that the JVM is fee to do whatever it wants - line B may observe what A did or it may not observe what A did. If you run it 5000 times, it may show you what A did all 5000 times, and then tomorrow when you have the important demo, it no longer does that.

Your code must function properly regardless of the 'choice' the JVM makes; you cannot ask the JVM what choice it has made/will make and you can't force it to make a choice other than by establishing HappensBefore which is the one and only way the JMM dictates you can guarantee observability. You can't write unit tests because without HB(A, B) it is still possible that B always observes the changes A makes. Today. On your machine. With this song on your music player. With this full moon. Who knows what happens tomorrow? When the song ends? When you run this on a different VM? Or a different version.

A few important facts about HB:

So, how do you establish that all important HB? There's a list of explicit actions that establish it in the JMM. That is the only way. Here's the important parts of that list:

Your mistakes

You make a change to unstartedRunnables without anything that can possibly establish HB. That means the change you make here may not be observable from anywhere. It doesn't matter that you can synchronized (this) elsewhere; both the writes and the reads need to use synchronized in other to get the HB established by synchronized.

Dangerous locking

this tends to be 'public', and locks should never be public unless extensively documented and considered part of your API. If that's too long to remember, 'locks should never be public' is an acceptable shorthand for that if you want to simplify things. You have a private field named lock right there - all those sync blocks should be synchronized (lock) instead!

For the same reason public fields are a bad idea, public locks are a bad idea.

Badly reinventing wheels

If you are getting the sense 'wait.. that means.. any concurrency code is utterly untestable and that means it is impossible to it right!' you have the right idea. Hence, writing your own low-level concurrency tools is rocket science and you should be incredibly smart, and slave away spending hours per line, to do this. Mere mortals should generally not attempt it.

Instead, look at the java.util.concurrent package. What you're trying to do? It has classes that do exactly that, and they were written in that extremely plodding, careful manner, and have been used and tested by millions of people.

executeUntilDeplated

I don't know what that code is meant to do, but it doesn't appear to do anything useful. In the javadoc it says:

If the thread has been interrupted (maybe shutdown of application)

This is wrong. interruptions do not occur due to shutdown of an app. Threads just end, they don't throw InterruptedExceptions. Only one thing causes InterruptedException: t.interrupt(). And nothing in the core java packages call that (except a select few execution pool things in j.u.c and they document it extensively). It means what you want it to mean, in other words.

Reasons:
  • Blacklisted phrase (1): how do you
  • RegEx Blacklisted phrase (2.5): Can you answer
  • Long answer (-1):
  • Has code block (-0.5):
  • Contains question mark (0.5):
  • User mentioned (1): @marcinj
  • High reputation (-2):
Posted by: rzwitserloot