[concurrency-interest] Adding toString methods for task objects?

Joe Bowbeer joe.bowbeer at gmail.com
Wed Aug 9 13:10:32 EDT 2017


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.

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.

If Guava or other libraries want to enhance this, that can be their value
added.

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.

On Wed, Aug 9, 2017 at 9:19 AM Charles Munger <clm at google.com> wrote:

> I added similar toString implementations to guava's Future implementations:
>
> https://github.com/google/guava/commit/304c634d977127085b49f174fd1aafefd09b2bf5
>
> 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.
>
> 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?
>
> On Tue, Aug 8, 2017 at 6:58 PM, Joe Bowbeer <joe.bowbeer at gmail.com> wrote:
>
>> ​I would consider this to be an API change. For example, according to the
>> current javadoc, FutureTask's toString method is inherited from Object.
>>
>> 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.
>>
>> If any of the state the toString implementations are accessing is not
>> thread-safe, I'd also be concerned about that.
>>
>> If tasks need toString implementations, I prefer to write them.​
>>
>> However, I think it would be OK to introduce new convenience methods:
>> debugString()
>>
>> On Tue, Aug 8, 2017 at 6:36 PM, Martin Buchholz <martinrb at google.com>
>> wrote:
>>
>>> 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?
>>>
>>> Index: Executors.java
>>> ===================================================================
>>> RCS file:
>>> /export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/Executors.java,v
>>> retrieving revision 1.97
>>> diff -u -U 10 -r1.97 Executors.java
>>> --- Executors.java 15 Apr 2017 00:12:38 -0000 1.97
>>> +++ Executors.java 9 Aug 2017 01:34:25 -0000
>>> @@ -478,20 +478,24 @@
>>>          private final Runnable task;
>>>          private final T result;
>>>          RunnableAdapter(Runnable task, T result) {
>>>              this.task = task;
>>>              this.result = result;
>>>          }
>>>          public T call() {
>>>              task.run();
>>>              return result;
>>>          }
>>> +        public String toString() {
>>> +            return super.toString()
>>> +                + "[task=[" + task + "], result=[" + result + "]]";
>>> +        }
>>>      }
>>>
>>>      /**
>>>       * A callable that runs under established access control settings.
>>>       */
>>>      private static final class PrivilegedCallable<T> implements
>>> Callable<T> {
>>>          final Callable<T> task;
>>>          final AccessControlContext acc;
>>>
>>>          PrivilegedCallable(Callable<T> task) {
>>> @@ -504,20 +508,24 @@
>>>                  return AccessController.doPrivileged(
>>>                      new PrivilegedExceptionAction<T>() {
>>>                          public T run() throws Exception {
>>>                              return task.call();
>>>                          }
>>>                      }, acc);
>>>              } catch (PrivilegedActionException e) {
>>>                  throw e.getException();
>>>              }
>>>          }
>>> +
>>> +        public String toString() {
>>> +            return super.toString() + "[task=[" + task + "]]";
>>> +        }
>>>      }
>>>
>>>      /**
>>>       * A callable that runs under established access control settings
>>> and
>>>       * current ClassLoader.
>>>       */
>>>      private static final class
>>> PrivilegedCallableUsingCurrentClassLoader<T>
>>>              implements Callable<T> {
>>>          final Callable<T> task;
>>>          final AccessControlContext acc;
>>> @@ -556,20 +564,24 @@
>>>                                  } finally {
>>>                                      t.setContextClassLoader(cl);
>>>                                  }
>>>                              }
>>>                          }
>>>                      }, acc);
>>>              } catch (PrivilegedActionException e) {
>>>                  throw e.getException();
>>>              }
>>>          }
>>> +
>>> +        public String toString() {
>>> +            return super.toString() + "[task=[" + task + "]]";
>>> +        }
>>>      }
>>>
>>>      /**
>>>       * The default thread factory.
>>>       */
>>>      private static class DefaultThreadFactory implements ThreadFactory {
>>>          private static final AtomicInteger poolNumber = new
>>> AtomicInteger(1);
>>>          private final ThreadGroup group;
>>>          private final AtomicInteger threadNumber = new AtomicInteger(1);
>>>          private final String namePrefix;
>>> Index: ForkJoinTask.java
>>> ===================================================================
>>> RCS file:
>>> /export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/ForkJoinTask.java,v
>>> retrieving revision 1.115
>>> diff -u -U 10 -r1.115 ForkJoinTask.java
>>> --- ForkJoinTask.java 19 Apr 2017 23:45:51 -0000 1.115
>>> +++ ForkJoinTask.java 9 Aug 2017 01:34:25 -0000
>>> @@ -1339,37 +1339,43 @@
>>>          T result;
>>>          AdaptedRunnable(Runnable runnable, T result) {
>>>              if (runnable == null) throw new NullPointerException();
>>>              this.runnable = runnable;
>>>              this.result = result; // OK to set this even before
>>> completion
>>>          }
>>>          public final T getRawResult() { return result; }
>>>          public final void setRawResult(T v) { result = v; }
>>>          public final boolean exec() { runnable.run(); return true; }
>>>          public final void run() { invoke(); }
>>> +        public String toString() {
>>> +            return super.toString() + "[task=[" + runnable + "],
>>> result=[" + result + "]]";
>>> +        }
>>>          private static final long serialVersionUID =
>>> 5232453952276885070L;
>>>      }
>>>
>>>      /**
>>>       * Adapter for Runnables without results.
>>>       */
>>>      static final class AdaptedRunnableAction extends ForkJoinTask<Void>
>>>          implements RunnableFuture<Void> {
>>>          final Runnable runnable;
>>>          AdaptedRunnableAction(Runnable runnable) {
>>>              if (runnable == null) throw new NullPointerException();
>>>              this.runnable = runnable;
>>>          }
>>>          public final Void getRawResult() { return null; }
>>>          public final void setRawResult(Void v) { }
>>>          public final boolean exec() { runnable.run(); return true; }
>>>          public final void run() { invoke(); }
>>> +        public String toString() {
>>> +            return super.toString() + "[task=[" + runnable + "]]";
>>> +        }
>>>          private static final long serialVersionUID =
>>> 5232453952276885070L;
>>>      }
>>>
>>>      /**
>>>       * Adapter for Runnables in which failure forces worker exception.
>>>       */
>>>      static final class RunnableExecuteAction extends ForkJoinTask<Void>
>>> {
>>>          final Runnable runnable;
>>>          RunnableExecuteAction(Runnable runnable) {
>>>              if (runnable == null) throw new NullPointerException();
>>> @@ -1402,20 +1408,23 @@
>>>                  result = callable.call();
>>>                  return true;
>>>              } catch (RuntimeException rex) {
>>>                  throw rex;
>>>              } catch (Exception ex) {
>>>                  throw new RuntimeException(ex);
>>>              }
>>>          }
>>>          public final void run() { invoke(); }
>>>          private static final long serialVersionUID =
>>> 2838392045355241008L;
>>> +        public String toString() {
>>> +            return super.toString() + "[task=[" + callable + "]]";
>>> +        }
>>>      }
>>>
>>>      /**
>>>       * Returns a new {@code ForkJoinTask} that performs the {@code run}
>>>       * method of the given {@code Runnable} as its action, and returns
>>>       * a null result upon {@link #join}.
>>>       *
>>>       * @param runnable the runnable action
>>>       * @return the task
>>>       */
>>> Index: FutureTask.java
>>> ===================================================================
>>> RCS file:
>>> /export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/FutureTask.java,v
>>> retrieving revision 1.118
>>> diff -u -U 10 -r1.118 FutureTask.java
>>> --- FutureTask.java 10 Sep 2016 04:06:51 -0000 1.118
>>> +++ FutureTask.java 9 Aug 2017 01:34:25 -0000
>>> @@ -444,20 +444,49 @@
>>>                              continue retry;
>>>                      }
>>>                      else if (!WAITERS.compareAndSet(this, q, s))
>>>                          continue retry;
>>>                  }
>>>                  break;
>>>              }
>>>          }
>>>      }
>>>
>>> +    public String toString() {
>>> +        String start = super.toString() + "[status=";
>>> +        try {
>>> +            switch (state) {
>>> +            case NEW:
>>> +            case COMPLETING:
>>> +                final Callable<?> callable = this.callable;
>>> +                if (callable != null) {
>>> +                    return start + "PENDING, task=[" + callable + "]]";
>>> +                } else {
>>> +                    return start + "PENDING]";
>>> +                }
>>> +            case NORMAL:
>>> +                return start + "SUCCESS, result=[" + outcome + "]]";
>>> +            case EXCEPTIONAL:
>>> +                return start + "FAILURE, cause=[" + outcome + "]]";
>>> +            case CANCELLED:
>>> +            case INTERRUPTING:
>>> +            case INTERRUPTED:
>>> +                return start + "CANCELLED]";
>>> +            default:
>>> +                throw new IllegalStateException();
>>> +            }
>>> +        } catch (RuntimeException thrownFromToString) {
>>> +            return start + "UNKNOWN, cause=["
>>> +                + thrownFromToString.getClass() + "]";
>>> +        }
>>> +    }
>>> +
>>>      // VarHandle mechanics
>>>      private static final VarHandle STATE;
>>>      private static final VarHandle RUNNER;
>>>      private static final VarHandle WAITERS;
>>>      static {
>>>          try {
>>>              MethodHandles.Lookup l = MethodHandles.lookup();
>>>              STATE = l.findVarHandle(FutureTask.class, "state",
>>> int.class);
>>>              RUNNER = l.findVarHandle(FutureTask.class, "runner",
>>> Thread.class);
>>>              WAITERS = l.findVarHandle(FutureTask.class, "waiters",
>>> WaitNode.class);
>>>
>>>
>>> _______________________________________________
>>> Concurrency-interest mailing list
>>> Concurrency-interest at cs.oswego.edu
>>> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cs.oswego.edu/pipermail/concurrency-interest/attachments/20170809/7f1146e3/attachment-0001.html>


More information about the Concurrency-interest mailing list