RFR: 6206780 (str) Forwarding append methods in String{Buffer, Builder} are inconsistent (original) (raw)

Peter Levart peter.levart at gmail.com
Sun Oct 7 22:17:12 UTC 2012


On 10/07/2012 08:42 PM, Alan Bateman wrote:

On 06/10/2012 01:52, Jim Gish wrote:

I've revamped the previous proposed change. Please review the update at http://cr.openjdk.java.net/~jgish/Bug6206780-sb-append-forward/ <http://cr.openjdk.java.net/%7Ejgish/Bug6206780-sb-append-forward/>

This revision contains the following additions/changes: 1. consistent usage of @Overrides 2. comments on unsynchronized methods depending on other synchronized methods 3. overall more code coverage of insert, append, indexOf, lastIndexOf, etc. for both StringBuffer and StringBuilder 4. testing of StringBuffer to ensure that all public un-synchronized methods are actually synchronized along their call chain. the StringBuffer/TestSynchronization class uses reflection to decide which methods to test and could be (with a bit of work on method parameter setup), be used to test any class for these synchronization properties. The motivation for this test is to provide some degree of assurance that modifications to StringBuffer, particularly addition of new methods, do not break the synchronization contract. The code also includes a self-test to guard against breaking the test program itself. I'm skimmed over this and StringBuffer looks a lot better, thanks for reverting back and adding the comments. I don't have time to review the tests just now but it looks like you've added a test of useful tests. I did look at TestSynchronization.testSynchronized and the approach looks right. The timeout is short so there will periodically be false positives but that's okay as increasing the timeout will slow down the test (the main thing I was looking for was the potential for false negatives). -Alan

Hi Jim,

Here's a way to test for synchronization deterministically. And it takes as much time as the testing methods runs (no timeouts):

public class SyncTest { public static class MyClass { public synchronized void synchronized_method() throws InterruptedException { Thread.sleep(1000); }

     public void not_synchronized_method() throws InterruptedException
     {
         Thread.sleep(1000);
     }

     public void indirectly_synchronized_method() throws 

InterruptedException { Thread.sleep(100); synchronized_method(); } }

 private static boolean isSynchronized(Method m, Object target)
 {
     Thread t = new Thread(new InvokeTask(m, target));

     Boolean isSynchronized = null;

     synchronized (target)
     {
         t.start();

         while (isSynchronized == null)
         {
             switch (t.getState())
             {
                 case NEW:
                 case RUNNABLE:
                 case WAITING:
                 case TIMED_WAITING:
                     Thread.yield();
                     break;
                 case BLOCKED:
                     isSynchronized = true;
                     break;
                 case TERMINATED:
                     isSynchronized = false;
                     break;
             }
         }
     }

     try
     {
         t.join();
     }
     catch (InterruptedException ex)
     {
         ex.printStackTrace();
     }

     return isSynchronized;
 }

 public static void main(String[] args)
 {
     Method[] methods = MyClass.class.getDeclaredMethods();
     Object target = new MyClass();

     for (Method m : methods)
     {
         System.out.println(m + " - isSynchronized: " + 

isSynchronized(m, target));; } }

 static class InvokeTask implements Runnable
 {
     private final Method m;
     private final Object target;
     private final Object[] args;

     InvokeTask(Method m, Object target, Object...args)
     {
         this.m = m;
         this.target = target;
         this.args = args;
     }

     @Override
     public void run()
     {
         try
         {
             m.invoke(target, args);
         }
         catch (Exception e)
         {
             e.printStackTrace();
         }
     }
 }

}

Regards, Peter



More information about the core-libs-dev mailing list