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

Jim Gish jim.gish at oracle.com
Wed Oct 10 20:19:24 UTC 2012

Thank you, Peter.

Alan -- could you please do a final review with the changes I made to the test and push it for me?

http://cr.openjdk.java.net/~jgish/Bug6206780-sb-append-forward/

Thanks, Jim

On 10/07/2012 06:17 PM, Peter Levart wrote: > 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 synchronizedmethod() throws > InterruptedException > { > Thread.sleep(1000); > } >> public void notsynchronizedmethod() throws InterruptedException > { > Thread.sleep(1000); > } >> public void indirectlysynchronizedmethod() throws > InterruptedException > { > Thread.sleep(100); > synchronizedmethod(); > } > } >> 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 TIMEDWAITING: > 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 >

Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.gish at oracle.com

More information about the core-libs-dev mailing list