<div dir="auto">RunnableAdapter is an interesting case. I'd have most of the same concerns as before, to a slightly less extent. This would not be an API change, however.</div><div dir="auto"><br></div><div dir="auto">The style adopted by Java so far has been that j.u.c. objects don't by default reveal their internal state. It has been up to devs to opt in, based on their particular needs, and some devs rightly or wrongly rely on the format inherited from Object.</div><div dir="auto"><br></div><div dir="auto">If Guava or other libraries want to enhance this, that can be their value added.</div><div dir="auto"><br></div><div dir="auto">By the way, adding to my previous list of concerns, I'd also be concerned about leaking PI in log messages. This is related to my concern about the wrapped toString impl. being designed for use in a different context.</div><div><br><div class="gmail_quote"><div>On Wed, Aug 9, 2017 at 9:19 AM Charles Munger <<a href="mailto:clm@google.com">clm@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>I added similar toString implementations to guava's Future implementations:<div><a href="https://github.com/google/guava/commit/304c634d977127085b49f174fd1aafefd09b2bf5" target="_blank">https://github.com/google/guava/commit/304c634d977127085b49f174fd1aafefd09b2bf5</a><br></div><div><br></div><div>The attraction of toString on common Future implementations is that propagating toString throughout a graph of transformed Futures lets you see what a task is doing before it completes, either in a TimeoutException message or the toString of LockSupport.getBlocker. </div><div><br></div><div>Do you feel that adding toString to RunnableAdapter (and similar classes, that are only exposed in public API as interfaces) would also constitute an API change?</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 8, 2017 at 6:58 PM, Joe Bowbeer <span><<a href="mailto:joe.bowbeer@gmail.com" target="_blank">joe.bowbeer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="gmail_default" style="font-size:small">​I would consider this to be an API change. For example, according to the current javadoc, FutureTask's toString method is inherited from Object.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Practically speaking, I'd be concerned about unexpected and unwanted changes to the output/log statements of programs that have up until now been working fine. In addition to generating a lot of output that may be unwanted, this may also generate exceptions. For example, PrivilegedCallable.toString calls the toString method of the wrapped Callable, which may never have been called previously (bam!), and therefore might throw an exception. Or, the toString method of the wrapped callable may have been designed to be called in a different context.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">If any of the state the toString implementations are accessing is not thread-safe, I'd also be concerned about that.<br></div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">If tasks need toString implementations, I prefer to write them.​</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">However, I think it would be OK to introduce new convenience methods: debugString()</div></div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="m_-4557839192558239390h5">On Tue, Aug 8, 2017 at 6:36 PM, Martin Buchholz <span><<a href="mailto:martinrb@google.com" target="_blank">martinrb@google.com</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="m_-4557839192558239390h5"><div>My colleague Charles Munger suggests adding toString methods to various task classes in j.u.c.  Obviously this makes it easier to debug large programs that have gone wrong.  But it obviously also is a behavior change that has the potential for trouble, even when everyone is behaving nicely, since toString may have the effect of generating an unbounded output.  I almost started down this road myself a few years ago but chickened out.  What do you think?<div><br></div><div><div>Index: Executors.java</div><div>===================================================================</div><div>RCS file: /export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/Executors.java,v</div><div>retrieving revision 1.97</div><div>diff -u -U 10 -r1.97 Executors.java</div><div>--- Executors.java<span style="white-space:pre-wrap"> </span>15 Apr 2017 00:12:38 -0000<span style="white-space:pre-wrap">      </span>1.97</div><div>+++ Executors.java<span style="white-space:pre-wrap">   </span>9 Aug 2017 01:34:25 -0000</div><div>@@ -478,20 +478,24 @@</div><div>         private final Runnable task;</div><div>         private final T result;</div><div>         RunnableAdapter(Runnable task, T result) {</div><div>             this.task = task;</div><div>             this.result = result;</div><div>         }</div><div>         public T call() {</div><div>             task.run();</div><div>             return result;</div><div>         }</div><div>+        public String toString() {</div><div>+            return super.toString()</div><div>+                + "[task=[" + task + "], result=[" + result + "]]";</div><div>+        }</div><div>     }</div><div> </div><div>     /**</div><div>      * A callable that runs under established access control settings.</div><div>      */</div><div>     private static final class PrivilegedCallable<T> implements Callable<T> {</div><div>         final Callable<T> task;</div><div>         final AccessControlContext acc;</div><div> </div><div>         PrivilegedCallable(Callable<T> task) {</div><div>@@ -504,20 +508,24 @@</div><div>                 return AccessController.doPrivileged(</div><div>                     new PrivilegedExceptionAction<T>() {</div><div>                         public T run() throws Exception {</div><div>                             return task.call();</div><div>                         }</div><div>                     }, acc);</div><div>             } catch (PrivilegedActionException e) {</div><div>                 throw e.getException();</div><div>             }</div><div>         }</div><div>+</div><div>+        public String toString() {</div><div>+            return super.toString() + "[task=[" + task + "]]";</div><div>+        }</div><div>     }</div><div> </div><div>     /**</div><div>      * A callable that runs under established access control settings and</div><div>      * current ClassLoader.</div><div>      */</div><div>     private static final class PrivilegedCallableUsingCurrentClassLoader<T></div><div>             implements Callable<T> {</div><div>         final Callable<T> task;</div><div>         final AccessControlContext acc;</div><div>@@ -556,20 +564,24 @@</div><div>                                 } finally {</div><div>                                     t.setContextClassLoader(cl);</div><div>                                 }</div><div>                             }</div><div>                         }</div><div>                     }, acc);</div><div>             } catch (PrivilegedActionException e) {</div><div>                 throw e.getException();</div><div>             }</div><div>         }</div><div>+</div><div>+        public String toString() {</div><div>+            return super.toString() + "[task=[" + task + "]]";</div><div>+        }</div><div>     }</div><div> </div><div>     /**</div><div>      * The default thread factory.</div><div>      */</div><div>     private static class DefaultThreadFactory implements ThreadFactory {</div><div>         private static final AtomicInteger poolNumber = new AtomicInteger(1);</div><div>         private final ThreadGroup group;</div><div>         private final AtomicInteger threadNumber = new AtomicInteger(1);</div><div>         private final String namePrefix;</div><div>Index: ForkJoinTask.java</div><div>===================================================================</div><div>RCS file: /export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/ForkJoinTask.java,v</div><div>retrieving revision 1.115</div><div>diff -u -U 10 -r1.115 ForkJoinTask.java</div><div>--- ForkJoinTask.java<span style="white-space:pre-wrap">    </span>19 Apr 2017 23:45:51 -0000<span style="white-space:pre-wrap">      </span>1.115</div><div>+++ ForkJoinTask.java<span style="white-space:pre-wrap">       </span>9 Aug 2017 01:34:25 -0000</div><div>@@ -1339,37 +1339,43 @@</div><div>         T result;</div><div>         AdaptedRunnable(Runnable runnable, T result) {</div><div>             if (runnable == null) throw new NullPointerException();</div><div>             this.runnable = runnable;</div><div>             this.result = result; // OK to set this even before completion</div><div>         }</div><div>         public final T getRawResult() { return result; }</div><div>         public final void setRawResult(T v) { result = v; }</div><div>         public final boolean exec() { runnable.run(); return true; }</div><div>         public final void run() { invoke(); }</div><div>+        public String toString() {</div><div>+            return super.toString() + "[task=[" + runnable + "], result=[" + result + "]]";</div><div>+        }</div><div>         private static final long serialVersionUID = 5232453952276885070L;</div><div>     }</div><div> </div><div>     /**</div><div>      * Adapter for Runnables without results.</div><div>      */</div><div>     static final class AdaptedRunnableAction extends ForkJoinTask<Void></div><div>         implements RunnableFuture<Void> {</div><div>         final Runnable runnable;</div><div>         AdaptedRunnableAction(Runnable runnable) {</div><div>             if (runnable == null) throw new NullPointerException();</div><div>             this.runnable = runnable;</div><div>         }</div><div>         public final Void getRawResult() { return null; }</div><div>         public final void setRawResult(Void v) { }</div><div>         public final boolean exec() { runnable.run(); return true; }</div><div>         public final void run() { invoke(); }</div><div>+        public String toString() {</div><div>+            return super.toString() + "[task=[" + runnable + "]]";</div><div>+        }</div><div>         private static final long serialVersionUID = 5232453952276885070L;</div><div>     }</div><div> </div><div>     /**</div><div>      * Adapter for Runnables in which failure forces worker exception.</div><div>      */</div><div>     static final class RunnableExecuteAction extends ForkJoinTask<Void> {</div><div>         final Runnable runnable;</div><div>         RunnableExecuteAction(Runnable runnable) {</div><div>             if (runnable == null) throw new NullPointerException();</div><div>@@ -1402,20 +1408,23 @@</div><div>                 result = callable.call();</div><div>                 return true;</div><div>             } catch (RuntimeException rex) {</div><div>                 throw rex;</div><div>             } catch (Exception ex) {</div><div>                 throw new RuntimeException(ex);</div><div>             }</div><div>         }</div><div>         public final void run() { invoke(); }</div><div>         private static final long serialVersionUID = 2838392045355241008L;</div><div>+        public String toString() {</div><div>+            return super.toString() + "[task=[" + callable + "]]";</div><div>+        }</div><div>     }</div><div> </div><div>     /**</div><div>      * Returns a new {@code ForkJoinTask} that performs the {@code run}</div><div>      * method of the given {@code Runnable} as its action, and returns</div><div>      * a null result upon {@link #join}.</div><div>      *</div><div>      * @param runnable the runnable action</div><div>      * @return the task</div><div>      */</div><div>Index: FutureTask.java</div><div>===================================================================</div><div>RCS file: /export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/FutureTask.java,v</div><div>retrieving revision 1.118</div><div>diff -u -U 10 -r1.118 FutureTask.java</div><div>--- FutureTask.java<span style="white-space:pre-wrap">   </span>10 Sep 2016 04:06:51 -0000<span style="white-space:pre-wrap">      </span>1.118</div><div>+++ FutureTask.java<span style="white-space:pre-wrap"> </span>9 Aug 2017 01:34:25 -0000</div><div>@@ -444,20 +444,49 @@</div><div>                             continue retry;</div><div>                     }</div><div>                     else if (!WAITERS.compareAndSet(this, q, s))</div><div>                         continue retry;</div><div>                 }</div><div>                 break;</div><div>             }</div><div>         }</div><div>     }</div><div> </div><div>+    public String toString() {</div><div>+        String start = super.toString() + "[status=";</div><div>+        try {</div><div>+            switch (state) {</div><div>+            case NEW:</div><div>+            case COMPLETING:</div><div>+                final Callable<?> callable = this.callable;</div><div>+                if (callable != null) {</div><div>+                    return start + "PENDING, task=[" + callable + "]]";</div><div>+                } else {</div><div>+                    return start + "PENDING]";</div><div>+                }</div><div>+            case NORMAL:</div><div>+                return start + "SUCCESS, result=[" + outcome + "]]";</div><div>+            case EXCEPTIONAL:</div><div>+                return start + "FAILURE, cause=[" + outcome + "]]";</div><div>+            case CANCELLED:</div><div>+            case INTERRUPTING:</div><div>+            case INTERRUPTED:</div><div>+                return start + "CANCELLED]";</div><div>+            default:</div><div>+                throw new IllegalStateException();</div><div>+            }</div><div>+        } catch (RuntimeException thrownFromToString) {</div><div>+            return start + "UNKNOWN, cause=["</div><div>+                + thrownFromToString.getClass() + "]";</div><div>+        }</div><div>+    }</div><div>+</div><div>     // VarHandle mechanics</div><div>     private static final VarHandle STATE;</div><div>     private static final VarHandle RUNNER;</div><div>     private static final VarHandle WAITERS;</div><div>     static {</div><div>         try {</div><div>             MethodHandles.Lookup l = MethodHandles.lookup();</div><div>             STATE = l.findVarHandle(FutureTask.class, "state", int.class);</div><div>             RUNNER = l.findVarHandle(FutureTask.class, "runner", Thread.class);</div><div>             WAITERS = l.findVarHandle(FutureTask.class, "waiters", WaitNode.class);</div></div><div><br></div></div>
<br></div></div>_______________________________________________<br>
Concurrency-interest mailing list<br>
<a href="mailto:Concurrency-interest@cs.oswego.edu" target="_blank">Concurrency-interest@cs.oswego.edu</a><br>
<a href="http://cs.oswego.edu/mailman/listinfo/concurrency-interest" rel="noreferrer" target="_blank">http://cs.oswego.edu/mailman/listinfo/concurrency-interest</a><br>
<br></blockquote></div><br></div>
</blockquote></div><br></div>
</blockquote></div></div>