6419880 close() hangs running consumeronnv_40

3 files changed, 100 insertions, 23 deletions

diff --git a/usr/src/lib/libdtrace_jni/java/src/org/opensolaris/os/dtrace/Consumer.java b/usr/src/lib/libdtrace_jni/java/src/org/opensolaris/os/dtrace/Consumer.java
index 6b6b47681d..59270ba21a 100644
--- a/usr/src/lib/libdtrace_jni/java/src/org/opensolaris/os/dtrace/Consumer.java
+++ b/usr/src/lib/libdtrace_jni/java/src/org/opensolaris/os/dtrace/Consumer.java
@@ -418,11 +418,51 @@ public interface Consumer {
      * @throws IllegalStateException if called before {@link #go()} or
      * if {@code stop()} was already called
      * @see #go()
+     * @see #abort()
      * @see #close()
      */
     public void stop();
 
     /**
+     * Aborts the background thread started by {@link #go()}.  {@code
+     * abort()} is effectively the same as {@link #stop()} except that
+     * it does not block (i.e. it does not wait until the background
+     * thread actually stops).  {@link #isRunning()} is likely {@code
+     * true} immediately after a call to {@code abort()}, since an
+     * aborted consumer stops at a time specified as later.
+     * Specifically, a call to {@code abort()} stops tracing just before
+     * the next {@link ConsumerListener#intervalBegan(ConsumerEvent e)
+     * intervalBegan()} event and stops consuming probe data by the
+     * subsequent {@link ConsumerListener#intervalEnded(ConsumerEvent e)
+     * intervalEnded()} event.  When the aborted consumer actually
+     * stops, listeners are notified in the {@link
+     * ConsumerListener#consumerStopped(ConsumerEvent e)
+     * consumerStopped()} method, where it is convenient to {@link
+     * #close()} the stopped consumer after requesting the final
+     * aggregate.
+     * <p>
+     * The {@code abort()} and {@code stop()} methods have slightly
+     * different behavior when called <i>just after</i> {@code go()} but
+     * <i>before</i> the consumer actually starts running:  It is
+     * possible to {@code stop()} a consumer before it starts running
+     * (resulting in a {@code consumerStopped()} event without a
+     * matching {@code consumerStarted()} event), whereas an aborted
+     * consumer will not stop until after it starts running, when it
+     * completes a single interval (that interval does not include
+     * sleeping to wait for traced probe data).  Calling {@code abort()}
+     * before {@code go()} is legal and has the same effect as calling
+     * it after {@code go()} and before the consumer starts running.
+     * The last behavior follows from the design: You do not know the
+     * state of a consumer after calling {@code abort()}, nor is it
+     * necessary to know the state of a consumer before calling {@code
+     * abort()}.  That may be preferable, for example, when you want to
+     * abort a consumer opened and started in another thread.
+     *
+     * @see #stop()
+     */
+    public void abort();
+
+    /**
      * Closes an open consumer and releases the system resources it was
      * holding.  If the consumer is running, {@code close()} will {@link
      * #stop()} it automatically.  A closed consumer cannot be
diff --git a/usr/src/lib/libdtrace_jni/java/src/org/opensolaris/os/dtrace/ConsumerListener.java b/usr/src/lib/libdtrace_jni/java/src/org/opensolaris/os/dtrace/ConsumerListener.java
index d207705678..7d0ffed97a 100644
--- a/usr/src/lib/libdtrace_jni/java/src/org/opensolaris/os/dtrace/ConsumerListener.java
+++ b/usr/src/lib/libdtrace_jni/java/src/org/opensolaris/os/dtrace/ConsumerListener.java
@@ -90,16 +90,17 @@ public interface ConsumerListener extends EventListener {
     /**
      * Called once when the source {@link Consumer} is stopped,
      * indicating that this listener should expect no further events.
-     * Called only if there was a prior call to {@link
-     * #consumerStarted(ConsumerEvent e) consumerStarted()}, that is,
-     * only if the consumer was successfully started by a call to {@link
-     * Consumer#go()}.  Guaranteed to be called whether the consumer was
-     * stopped by request (via {@link Consumer#stop()}), terminated
-     * normally as a result of the DTrace {@code exit()} action or the
-     * completion of all target processes, or terminated abnormally
-     * because of an exception.  It is necessary to call {@link
-     * Consumer#close()} to release any system resources still held by
-     * the stopped consumer.
+     * Guaranteed to be called whether the consumer was stopped by
+     * request (by calling {@link Consumer#stop()} or {@link
+     * Consumer#abort()}), terminated normally as a result of the DTrace
+     * {@code exit()} action (see <a
+     * href=http://docs.sun.com/app/docs/doc/817-6223/6mlkidlhm?a=view>
+     * <tt>exit()</tt></a> in the <b>Special Actions</b> section of the
+     * <b>Actions and Subroutines</b> chapter of the <i>Solaris Dynamic
+     * Tracing Guide</i>) or after the completion of all target
+     * processes, or terminated abnormally because of an exception.  It
+     * is necessary to call {@link Consumer#close()} to release any
+     * system resources still held by the stopped consumer.
      *
      * @see #consumerStarted(ConsumerEvent e)
      */
diff --git a/usr/src/lib/libdtrace_jni/java/src/org/opensolaris/os/dtrace/LocalConsumer.java b/usr/src/lib/libdtrace_jni/java/src/org/opensolaris/os/dtrace/LocalConsumer.java
index be0d54fef0..7cc72a019c 100644
--- a/usr/src/lib/libdtrace_jni/java/src/org/opensolaris/os/dtrace/LocalConsumer.java
+++ b/usr/src/lib/libdtrace_jni/java/src/org/opensolaris/os/dtrace/LocalConsumer.java
@@ -746,6 +746,13 @@ public class LocalConsumer implements Consumer {
 	t.start();
     }
 
+    /**
+     * @inheritDoc
+     *
+     * @throws IllegalThreadStateException if attempting to {@code
+     * stop()} a running consumer while holding the lock on that
+     * consumer
+     */
     public void
     stop()
     {
@@ -791,7 +798,7 @@ public class LocalConsumer implements Consumer {
 			throw new IllegalStateException(
 				"consumer already stopped");
 		    }
-		    logger.info("consumer already stopped");
+		    logger.fine("consumer already stopped");
 		    break;
 		case CLOSED:
 		    throw new IllegalStateException("consumer closed");
@@ -804,6 +811,12 @@ public class LocalConsumer implements Consumer {
 	}
 
 	if (running) {
+	    if (Thread.holdsLock(this)) {
+		throw new IllegalThreadStateException("The current " +
+			"thread cannot stop this LocalConsumer while " +
+			"holding the lock on this LocalConsumer");
+	    }
+
 	    //
 	    // Calls no libdtrace methods, so no synchronization is
 	    // needed.  Sets a native flag that causes the consumer
@@ -833,26 +846,49 @@ public class LocalConsumer implements Consumer {
 	}
     }
 
-    public synchronized void
+    public void
+    abort()
+    {
+	_interrupt();
+    }
+
+    /**
+     * @inheritDoc
+     *
+     * @throws IllegalThreadStateException if attempting to {@code
+     * close()} a running consumer while holding the lock on that
+     * consumer
+     */
+    public void
     close()
     {
-	if ((state == State.INIT) || (state == State.CLOSED)) {
-	    state = State.CLOSED;
-	    return;
+	synchronized (this) {
+	    if ((state == State.INIT) || (state == State.CLOSED)) {
+		state = State.CLOSED;
+		return;
+	    }
 	}
 
-	if ((state == State.STARTED) || (state == State.GO)) {
+	try {
 	    stop();
+	} catch (IllegalStateException e) {
+	    // ignore (we don't have synchronized state access because
+	    // it is illegal to call stop() while holding the lock on
+	    // this consumer)
 	}
 
-	synchronized (LocalConsumer.class) {
-	    _close();
-	}
-	_destroy();
-	state = State.CLOSED;
+	synchronized (this) {
+	    if (state != State.CLOSED) {
+		synchronized (LocalConsumer.class) {
+		    _close();
+		}
+		_destroy();
+		state = State.CLOSED;
 
-	if (logger.isLoggable(Level.INFO)) {
-	    logger.info("consumer table count: " + _openCount());
+		if (logger.isLoggable(Level.INFO)) {
+		    logger.info("consumer table count: " + _openCount());
+		}
+	    }
 	}
     }