Mutable Objects and Thread Boundaries

M

markspace

Lew said:
The rule of thumb I recommend: Keep critical sections to the minimum
length necessary to correctly coordinate concurrent access. That will
introduce tension against markspace's advice to simplify the
concurrent code by piling more into the critical section. Balancing
the two recommendations is a matter of art.


I'd recommend making the code as simple to maintain as possible, until
you are sure you have 64-core machines, and you know which locks need to
be optimized.

Balancing the two recommendations is not a matter of art, it's a matter
of profiling running code. ;) Pre-mature optimization is the root of
all evil. Don't do it until you're sure you have to and have the code
profiles in hand to prove it.
 
A

Alan Gutierrez

markspace said:
See, I'd go the other way. Even though I don't personally know any way
your thread is exposed to a client either, I'd still let an interrupt
kill the thread. Two reasons for this:

1) There may be some way a client can send a SIGHUP or similar through
the console, which the JVM might propagate to all threads, for an
orderly shutdown (kill -9 is disorderly and abrupt).

There is such a way to accept a SIGTERM, Runtime.addShutdownHook, but it
is my understanding that it is up to the developer to implement the
orderly shutdown of her own threads. There is no mention of threads
being shutdown with interrupt.

In the case of my library, you would call a publicly exposed shutdown
method that would terminate the worker thread in a fashion that
indicated an orderly shutdown.

So there are no interrupts sent by the virtual machine at shutdown.
Interrupting a thread is something done deliberately.

I got to musing about daemon threads, too. I began to wonder if the JVM
simply stopped executing statements, or if the overly ambitions stop,
start, ThreadDeath mechanism still played some part in shutdown.

This program below shows that no interrupt it sent to the main thread
and nothing propagates out of the daemon thread, on OSX Sun JVM and on
Linux running Open JDK. Compile and run and send a SIGTERM using Ctl+C
or kill and the program stops dead.

public class SetDaemon {
public static void main(String[] args) {
Thread thread = new Thread(new Runnable() {
public void run() {
try {
for (;;) {}
} catch (Throwable e) {
e.printStackTrace();
if (e instanceof Error) {
throw (Error) e;
}
throw (RuntimeException) e;
}
}
});
thread.setDaemon(true);
thread.start();
try {
Thread.sleep(Long.MAX_VALUE);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
}

2) Another programmer might assume that any thread can be killed with an
interrupt, and might be confused or frustrated when it can't. In this
light, it's just a bit of future proofing and courtesy to make your
thread work in the standard way.

Point taken. I'll throw an exception instead. I maintain that this tread
is part of the internal state of the library and sending it an interrupt
is a programming error, but as a programming error, it should throw an
exception the moment the error is detected.
 
A

Alan Gutierrez

ClassCastException said:
My usual way of handling an "unreachable" checked exception block is
"throw new Error();".

Thank you. I'll throw runtime exceptions when I catch an
InterruptedException I was not expecting, instead of retrying. I said as
much in my last response.
 
A

Alan Gutierrez

markspace said:
Brian Goetz calls this piggy-backing in JCIP. (He uses a volatile
variable as an example, which is why I was confused about synchronized
blocks.) And he says it's a dangerous thing to do because it's hard to
reason out these side effect correctly, and hard for maintenance
programmers to spot them as well. So to the OP and others, don't rely
on these side effects. Put all data in a synchronized block and you'll
be less likely to wonder what the heck is going on six months after your
wrote the code.

The OP is perfectly capable of reasoning out the usage of a BlockingQueue.
 
M

markspace

Alan said:
Point taken. I'll throw an exception instead. I maintain that this tread
is part of the internal state of the library and sending it an interrupt
is a programming error, but as a programming error, it should throw an
exception the moment the error is detected.


A good idea, I think. If you consider an interrupt to be an error, then
you certainly want to make note of any interrupts that you find.

Other ideas: log the error, but continue running anyway. This might be
problematic since you are the logging thread in this case. Consider
(also?) writing to stderr.

Another one: I've seen code that attempted an orderly shutdown on
SIGHUP, but if it caught a second one, then someone is really trying to
stop your process (probably a frustrated operator at the console mashing
the control-C button). On the first interrupt, continue running
(probably record the interrupt though) while attempting an orderly
shutdown. On the second interrupt, die immediately.

Just tossin' ideas out....
 
L

Lew

'InterruptedException' is not about SIGINT or other OS signals, but
about the Java method 'Thread#interrupt()'.
<http://download.oracle.com/docs/cd/E17409_01/javase/6/docs/api/java/
lang/Thread.html#interrupt()>
<http://download.oracle.com/docs/cd/E17409_01/javase/6/docs/api/java/
lang/InterruptedException.html>

There is excellent advice in /Java Concurrency in Practice/ (JCIP.
Part of the reason to catch it is that the JVM apparently can throw
spurious 'InterruptedException's.

Read JCIP (by Brian Goetz et al.).

Alan said:
There is such a way to accept a SIGTERM, Runtime.addShutdownHook, but it
is my understanding that it is up to the developer to implement the
orderly shutdown of her own threads. There is no mention of threads
being shutdown with interrupt.

In the case of my library, you would call a publicly exposed shutdown
method that would terminate the worker thread in a fashion that
indicated an orderly shutdown.

So there are no interrupts sent by the virtual machine at shutdown.
Interrupting a thread is something done deliberately.

Or not, if you get a spurious interruption.
I got to musing about daemon threads, too. I began to wonder if the JVM
simply stopped executing statements, or if the overly ambitions stop,
start, ThreadDeath mechanism still played some part in shutdown.

This program below shows that no interrupt it sent to the main thread
and  nothing propagates out of the daemon thread, on OSX Sun JVM and on
Linux running Open JDK. Compile and run and send a SIGTERM using Ctl+C

Ctrl-C sends SIGINT, not that that is relevant to
'InterruptedException'.
or kill and the program stops dead.

public class SetDaemon {
     public static void main(String[] args) {
         Thread thread = new Thread(new Runnable() {
             public void run() {
                 try {
                     for (;;) {}
                 } catch (Throwable e) {
                     e.printStackTrace();
                     if (e instanceof Error) {
                         throw (Error) e;
                     }
                     throw (RuntimeException) e;
                 }
             }
         });
         thread.setDaemon(true);
         thread.start();
         try {
             Thread.sleep(Long.MAX_VALUE);
         } catch (InterruptedException e) {
             e.printStackTrace();
         }
     }

}
2) Another programmer might assume that any thread can be killed with an
interrupt, and might be confused or frustrated when it can't.  In this
light, it's just a bit of future proofing and courtesy to make your
thread work in the standard way.

Alan said:
Point taken. I'll throw an exception instead. I maintain that this tread
is part of the internal state of the library and sending it an interrupt
is a programming error,

Or a spurious 'InterruptedException'.
but as a programming error, it should throw an
exception the moment the error is detected.

Unless it was a spurious exception.
 
M

markspace

Alan said:
The OP is perfectly capable of reasoning out the usage of a BlockingQueue.


I don't want to sound snarky or anything, but I think piggy-backing with
side effects is still something to be very careful of.

Will everyone who comes after you be just as aware of BlockigQueue's
semantics? And will you remember yourself at 3 am while trying to make
some last minute code changes?

Other than the obvious (the object being enqueued itself) if you rely on
any side effects, I'd document the heck out of them. E.g.:

package test;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingDeque;

/* Threadsafe */
public class PiggyBack {

private final BlockingQueue<String> queue =
new LinkedBlockingDeque<String>();

/* Guarded by queue */
private int received;

public int send( String s ) throws InterruptedException {
int retVal;
/* make atomic */
synchronized( this ) {
retVal = received += s.length();
}

/* received is guarded by this queue, not the synchronized
* block above.
* do not re-order the update to received below this line.
*/
queue.put( s );

return retVal;
}

public int getReceived() {
/* must access queue to provide thread safety.
* field "received" is guarded by the queue via piggy-backing.
*/
queue.peek();
return received;
}

public String recv() {
return queue.poll();
}
}

This may be a silly example, but that's the sort of thing I'm talking about.
 
A

Alan Gutierrez

Lew said:
'InterruptedException' is not about SIGINT or other OS signals, but
about the Java method 'Thread#interrupt()'.
<http://download.oracle.com/docs/cd/E17409_01/javase/6/docs/api/java/
lang/Thread.html#interrupt()>
<http://download.oracle.com/docs/cd/E17409_01/javase/6/docs/api/java/
lang/InterruptedException.html>

There is excellent advice in /Java Concurrency in Practice/ (JCIP.
Part of the reason to catch it is that the JVM apparently can throw
spurious 'InterruptedException's.

I think you are confusing spurious wakeup, explicitly mentioned in the
documentation for `Object.wait`. There is no such thing as spurious
interrupts.
 
D

Daniel Pitts

'InterruptedException' is not about SIGINT or other OS signals, but
about the Java method 'Thread#interrupt()'.
<http://download.oracle.com/docs/cd/E17409_01/javase/6/docs/api/java/
lang/Thread.html#interrupt()>
<http://download.oracle.com/docs/cd/E17409_01/javase/6/docs/api/java/
lang/InterruptedException.html>

There is excellent advice in /Java Concurrency in Practice/ (JCIP.
Part of the reason to catch it is that the JVM apparently can throw
spurious 'InterruptedException's.

Read JCIP (by Brian Goetz et al.).
I think you've misinterpreted something. There are possible spurious
wake-ups, but there are *not* spurious interrupts. Object.wait(...) may
*return* before the conditions are met, but will not through
InterruptedException unless there is a true interrupt.

BTW, there *is* an evil way to kill other threads, whether they handle
interrupted or not. Thread.stop(throwable);

<http://download.oracle.com/docs/cd/...va/lang/Thread.html#stop(java.lang.Throwable)>
 
A

Alan Gutierrez

markspace said:
I don't want to sound snarky or anything, but I think piggy-backing with
side effects is still something to be very careful of.
Will everyone who comes after you be just as aware of BlockigQueue's
semantics? And will you remember yourself at 3 am while trying to make
some last minute code changes?

Well, actually, I'd probably leave quite a few line comments, with a
link to this thread in Google Groups (Hello, future me!) My expectation
was that the writes to the `List` I put `offer` to the `BlockingQueue`
happen before the other thread `take`s from the queue and reads. I'm
coming out of this thread with a workable understanding of "happens
before" which I plan to not forget, or will make sure I remember before
bombing around an delicate concurrent code.

Again, your point is taken, tough. I'm expecting that high level
abstractions like `BlockingQueue` help me not have to think about things
and use them in lieu of `synchronized` blocks. Use of one way volatile
reads and writes is definately more difficult to reason out than the use
of `synchronized` blocks.

The point of this thread was to get a sanity check that I didn't need
extra synchronization, that the `BlockingQueue` did what I expected it
to do. Now that I know that it does, I'll use it instead of trying to
reason out my own form of queuing.

Also, I'll note that I'm implementing a write ahead log, so I do expect
this bit of code to be the complicated bit, so if it is 3 am, I'll get
some sleep before trying to page the project back into my head. This is
in its own project so I can isolate the problem and test it. The
complexity budget for the project is spent entirely on getting the queue
and file append to work correctly.

It would be a waste of complexity if this was part of a login form.
 
L

Lew

Daniel said:
I think you've misinterpreted something. There are possible spurious
wake-ups, but there are *not* spurious interrupts. Object.wait(...) may

You're absolutely right - my mistake.
*return* before the conditions are met, but will not through
InterruptedException unless there is a true interrupt.

The fact remains that 'InterruptedException' is not about SIGINT or other OS
signals but about response to an 'interrupted' condition.

JCIP claims that there are two possible correct responses to receiving an
'InterruptedException' - re-interrupt the thread itself (p. 142):
"- Propagate the exception...
"- Restore the interruption status ..."

and the example on p. 144 is specifically about 'BlockingQueue#take()'. He
most emphatically does not recommend any of the approaches recommended so far
in this discussion.
 
C

ClassCastException

class Mutable {

private final List<String> names;

public Mutable() {
names = new List<String>();
names.add("Joe");
names.add("Jim");
}

public List<String> getNames() {
return names;
}
}

This class would be immutable except for the fact that the private final
variable "names" escapes and could be modified.

That, and the fact that it won't compile. :)

Try this version for immutable goodness:

public class Immutable {

private final List<String> names;

public Mutable() {
List<String> n = new LinkedList<String>();
n.add("Joe");
n.add("Jim");
names = Collections.unmodifiableList(n);
}

public List<String> getNames() {
return names;
}
}
 
C

ClassCastException

You're absolutely right - my mistake.


The fact remains that 'InterruptedException' is not about SIGINT or
other OS signals but about response to an 'interrupted' condition.

JCIP claims that there are two possible correct responses to receiving
an 'InterruptedException' - re-interrupt the thread itself (p. 142): "-
Propagate the exception...
"- Restore the interruption status ..."

and the example on p. 144 is specifically about 'BlockingQueue#take()'.
He most emphatically does not recommend any of the approaches
recommended so far in this discussion.

JCIP does not consider these to be acceptable?

"- Exit the thread by returning from its run() method"
"- Exit the thread by throwing another, unhandled exception, perhaps with
the InterruptedException as its cause"

That seems odd, since the first one is necessary if a thread is to
actually interrupt while not producing any stack trace barf on the
console (for cases when the interruption is NOT a bug) and the second is
necessary if you want good layer decoupling and requiring every
intermediate caller to handle or explicitly pass InterruptedException is
poor layer decoupling. Then the standard method of "transforming" or
wrapping layer-specific exceptions at layer borders seems indicated. In
particular, in the case that the InterruptedException should not happen
in production, you want to log it and either recover or abort, either the
interrupted thread or the whole process/servlet/whatever.
 
M

markspace

Lew said:
He most emphatically does not recommend any of the approaches
recommended so far in this discussion.


JCIP, p 143:

"If you don't want to or cannot propagate InterruptedExecptions (perhaps
because your task is defined by a Runnable), you need to find another
way to do preserve the interruption request. The standard way to do
this is to restore the Interrupted status by calling interrupt() again.
What you should _not_ do is swallow the InterruptedException by
catching it and doing nothing in the catch block, *unless your code is
actually implementing the interruption policy for a thread.*"


*Emphasis* mine. So it's ok to swallow an InterruptedException, if
you're implementing the thread's response to said interruption request.
Which is what the OP is doing. He's deciding between ignoring,
exiting, or logging and exiting. Whatever he decides, that will be his
thread's interruption policy.

Also, on page 141 he shows an example of swallowing the exception, with
the very next act of the task being to exit the run() method of its
Thread. Again basically the same thing as what we're advising the OP to do.
 
M

markspace

ClassCastException said:
names = Collections.unmodifiableList(n);


Yeah, I messed-up with "new List()", but this line above isn't needed to
make this object immutable. It's immutable without it.
 
A

Alan Gutierrez

Lew said:
You're absolutely right - my mistake.


The fact remains that 'InterruptedException' is not about SIGINT or
other OS signals but about response to an 'interrupted' condition.

JCIP claims that there are two possible correct responses to receiving
an 'InterruptedException' - re-interrupt the thread itself (p. 142):
"- Propagate the exception...
"- Restore the interruption status ..."

and the example on p. 144 is specifically about 'BlockingQueue#take()'.
He most emphatically does not recommend any of the approaches
recommended so far in this discussion.

Like so:

http://www.ibm.com/developerworks/java/library/j-jtp05236.html

Then I believe OP really wants to make the worker a non-cancelable task,
if that is a valid option according to Goetz.
 
C

ClassCastException

Yeah, I messed-up with "new List()", but this line above isn't needed to
make this object immutable. It's immutable without it.

It isn't if the list escapes in a form where it can be modified. It is if
the getNames method wraps its return value using
Collections.unmodifiableList, but that has two effects different from
doing it in the constructor:

1. The wrapping is done every time the getNames method is called instead
of just once.
2. The list can be modified through the instance variable. Someone
modifying the class could change it from technically-immutable to
mutable, perhaps even without intending to. Making the list
unmodifiable through the ivar makes any such change throw an exception
that will lead to that programmer quickly realizing "oops". And then
deciding if they really want to lose the class's immutability, or
solve whatever they're trying to solve in some other way instead.
But if they make it mutable it will be a conscious decision to do so
and they will know to check the class's users to fix any code that
assumes instances are immutable and that now won't work.
 
A

Alan Gutierrez

ClassCastException said:
JCIP does not consider these to be acceptable?

"- Exit the thread by returning from its run() method"
"- Exit the thread by throwing another, unhandled exception, perhaps with
the InterruptedException as its cause"

That seems odd, since the first one is necessary if a thread is to
actually interrupt while not producing any stack trace barf on the
console (for cases when the interruption is NOT a bug) and the second is
necessary if you want good layer decoupling and requiring every
intermediate caller to handle or explicitly pass InterruptedException is
poor layer decoupling. Then the standard method of "transforming" or
wrapping layer-specific exceptions at layer borders seems indicated. In
particular, in the case that the InterruptedException should not happen
in production, you want to log it and either recover or abort, either the
interrupted thread or the whole process/servlet/whatever.

Thank you for considering the intent of the OP in your response.

It is my intent that this worker thread not throw any exceptions and
that all exceptions are considered catastrophic. The worker thread is
writing a write ahead log, so any error is met by shutting down the
thread to preserve the log, notifying waiting threads that it has
shutdown. Waiting threads are waiting within the write ahead log
library, which will detect the shutdown and throw an exception that says
that the write ahead log has gone away. Future calls to the write ahead
log throw exceptions. The absence of the log is unrecoverable.

The obvious way for a write ahead log to fail is for the disk to fill.
Thus, one of the messages you can send to the worker is to switch log
directories, so that long before the disk fills, you can tell the log to
write to a new disk. Why run the train to the end of the line, then put
it back on the tracks after you laid some more? Is that anyway to run a
railroad?

Thus, I have a universal strategy for faults, which is, if things are
not going as you expect, *stop* *writing* to the write ahead log, close
it, shutdown completely, and start the forensics to figure out what went
wrong. The rest of the system will come crashing down, but full disks,
and the like, will do that.

Which is why, sending it an interrupt is something that I would want to
know immediately, so that I'd be in a better position to determine which
bit of code is interrupting threads arbitrarily and excise it.

This discussion helped me see that continue after interrupt is
absolutely not what I want.

So, whether or not I reset the interrupted status on the way down the
drain is probably not that important, but I have no problem with adding
that one line of code.
 
A

Alan Gutierrez

markspace said:
JCIP, p 143:

"If you don't want to or cannot propagate InterruptedExecptions (perhaps
because your task is defined by a Runnable), you need to find another
way to do preserve the interruption request. The standard way to do
this is to restore the Interrupted status by calling interrupt() again.
What you should _not_ do is swallow the InterruptedException by
catching it and doing nothing in the catch block, *unless your code is
actually implementing the interruption policy for a thread.*"


*Emphasis* mine. So it's ok to swallow an InterruptedException, if
you're implementing the thread's response to said interruption request.
Which is what the OP is doing. He's deciding between ignoring,
exiting, or logging and exiting. Whatever he decides, that will be his
thread's interruption policy.

Just finished a longer response to CCE's message, where I explained my
strategy for faults in the thread, but the same sentiment, thanks for
considering the context of the OP (me) in the discussion. Thanks for
pointing out the exception to the rule as it applies to my question.

I laid out my fault-tolerance plans, because discussions about catch
blocks can sometimes follow the "can't see the forest for the trees"
reasoning.

If its not clear (its late here) I'm agreeing with you and CCE that the
bit about the interrupted state is good to know, but that the problem is
wider than that, and the chances that anyone is going to do anything
with the interrupted state flag are slim.
 
D

Daniel Pitts

You're absolutely right - my mistake.


The fact remains that 'InterruptedException' is not about SIGINT or
other OS signals but about response to an 'interrupted' condition.

JCIP claims that there are two possible correct responses to receiving
an 'InterruptedException' - re-interrupt the thread itself (p. 142):
"- Propagate the exception...
"- Restore the interruption status ..."
Actually, thats on page 93.

Page 143 does state:
"Only code that implements a thread's interruption policy may swallow an
interruption request."

So there are three possible responses:
1. Propagate the exception
2. Restore the interruption status.
3. Swallow the interrupt, but *only* if your code is part of the
thread lifecycle management.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,774
Messages
2,569,598
Members
45,151
Latest member
JaclynMarl
Top