<div dir="ltr">(getting emails out of order)<div><br></div><div>Forget my comment, I agree that Nitsan is on the money.  This could break even if reader and writer ran on same core and got scheduled in the "right" order.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 8, 2014 at 5:17 PM, Martin Thompson <span dir="ltr"><<a href="mailto:mjpt777@gmail.com" target="_blank">mjpt777@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 dir="ltr">Thanks Nitsan I think you are correct here. I'll have another look tomorrow morning after coffee :-)<br><div class="gmail_extra"><br><div class="gmail_quote">On 8 September 2014 21:53, Nitsan Wakart <span dir="ltr"><<a href="mailto:nitsanw@yahoo.com" target="_blank">nitsanw@yahoo.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 style="color:#000;background-color:#fff;font-family:HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif;font-size:10pt"><div>1    readCounter = getLongVolatile(READER_COUNTER_OFFSET);</div><div>2      writeCounter = getLongVolatile(WRITER_COUNTER_OFFSET);</div><div>3      if (readCounter == writeCounter && isFlagSet()) {</div><div>4           return false;</div><div>5       } else if (readCounter < writeCounter) {</div><div>6           putLongVolatile(READER_COUNTER_OFFSET, ++readCounter);</div><div>7       }</div><div><br></div><div>It is possible for 1,2 to execute and see 0, then have the writer thread blaze through it's loop and set the flag to 1before you hit 3.</div><div><span>
















</span></div><div>The HB relationship assumption is in the wrong order, you should read flag first and flag visibility necessitates the counter visibility.</div><div><div class="h5"> <div><br><br></div><div style="display:block"> <div style="font-family:HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif;font-size:10pt"> <div style="font-family:HelveticaNeue,Helvetica Neue,Helvetica,Arial,Lucida Grande,sans-serif;font-size:12pt"><div><div> <div dir="ltr"> <font face="Arial"> On Monday, September 8, 2014 10:23 PM, Martin Thompson <<a href="mailto:mjpt777@gmail.com" target="_blank">mjpt777@gmail.com</a>> wrote:<br> </font> </div>  <br><br> </div></div><div><div><div><div><div dir="ltr">The version below fails for me every time but at different iteration values.<div><br></div><div>Regards,</div><div>Martin...<br><div><br></div><div><div>import sun.misc.Unsafe;</div><div><br></div><div>import java.io.File;</div><div>import java.io.IOException;</div><div>import java.io.RandomAccessFile;</div><div>import java.lang.reflect.Field;</div><div>import java.nio.MappedByteBuffer;</div><div>import java.nio.channels.FileChannel;</div><div>import java.nio.file.Files;</div><div>import java.nio.file.StandardOpenOption;</div><div><br></div><div>class IpcBase implements AutoCloseable</div><div>{</div><div>    private static final long PADDING = 64;</div><div>    static final long READER_COUNTER_OFFSET = PADDING;</div><div>    static final long WRITER_COUNTER_OFFSET = READER_COUNTER_OFFSET + PADDING;</div><div>    static final long FLAG_OFFSET = WRITER_COUNTER_OFFSET + PADDING;</div><div>    static final long TOTAL_LENGTH = FLAG_OFFSET + PADDING;</div><div><br></div><div>    private static final Unsafe UNSAFE;</div><div><br></div><div>    static</div><div>    {</div><div>        try</div><div>        {</div><div>            final Field field =
 Unsafe.class.getDeclaredField("theUnsafe");</div><div>            field.setAccessible(true);</div><div>            UNSAFE = (Unsafe)field.get(null);</div><div>        }</div><div>        catch (final Exception ex)</div><div>        {</div><div>            throw new RuntimeException(ex);</div><div>        }</div><div>    }</div><div><br></div><div>    private final FileChannel channel;</div><div>    private final MappedByteBuffer buffer; // hold reference to prevent GC</div><div>    private final long
 baseAddress;</div><div><br></div><div>    public IpcBase(final File file) throws IOException</div><div>    {</div><div>        channel = new RandomAccessFile(file, "rw").getChannel();</div><div>        buffer = channel.map(FileChannel.MapMode.READ_WRITE, 0, TOTAL_LENGTH);</div><div>        baseAddress = ((sun.nio.ch.DirectBuffer)buffer).address();</div><div>    }</div><div><br></div><div>    final void putLongVolatile(final long offset, final long value)</div><div>    {</div><div>        UNSAFE.putLongVolatile(null, baseAddress + offset, value);</div><div>   
 }</div><div><br></div><div>    final long getLongVolatile(final long offset)</div><div>    {</div><div>        return UNSAFE.getLongVolatile(null, baseAddress + offset);</div><div>    }</div><div><br></div><div>    @Override</div><div>    public void close() throws Exception</div><div>    {</div><div>        channel.close();</div><div>    }</div><div>}</div><div><br></div><div>class Writer extends IpcBase</div><div>{</div><div>    long writerCounter;</div><div><br></div><div>    public Writer(final File file) throws IOException</div><div>    {</div><div>        super(file);</div><div>    }</div><div><br></div><div>    @Override</div><div>    public void close() throws Exception</div><div>    {</div><div>        putLongVolatile(FLAG_OFFSET, 1L);</div><div>        super.close();</div><div>    }</div><div><br></div><div>    public long write()</div><div>    {</div><div>        writerCounter =
 getLongVolatile(WRITER_COUNTER_OFFSET);</div><div>        putLongVolatile(WRITER_COUNTER_OFFSET, ++writerCounter);</div><div>        return writerCounter;</div><div>    }</div><div>}</div><div><br></div><div>class Reader extends IpcBase</div><div>{</div><div>    long readCounter;</div><div>    long writeCounter;</div><div><br></div><div>    public Reader(final File file) throws IOException</div><div>    {</div><div>        super(file);</div><div>    }</div><div><br></div><div>    public
 boolean read()</div><div>    {</div><div>        readCounter = getLongVolatile(READER_COUNTER_OFFSET);</div><div>        writeCounter = getLongVolatile(WRITER_COUNTER_OFFSET);</div><div>        if (readCounter == writeCounter && isFlagSet())</div><div>        {</div><div>            return false;</div><div>        }</div><div>        else if (readCounter < writeCounter)</div><div>        {</div><div>            putLongVolatile(READER_COUNTER_OFFSET, ++readCounter);</div><div>        }</div><div><br></div><div>        return true;</div><div>    }</div><div><br></div><div>    private boolean isFlagSet()</div><div>    {</div><div>        return 1L == getLongVolatile(FLAG_OFFSET);</div><div>    }</div><div>}</div><div><br></div><div>public class IpcBugTest</div><div>{</div><div>    public static int iteration = 0;</div><div><br></div><div>    public static void main(String[] args) throws Exception</div><div>    {</div><div>        final File file =
 createFile();</div><div><br></div><div>        for (int i = 0; i < 1_000_000; i++)</div><div>        {</div><div>            iteration = i;</div><div>            testIpc(file);</div><div>        }</div><div><br></div><div>        System.out.println("NO BUG HERE! :-)");</div><div>    }</div><div><br></div><div>    private static File createFile() throws IOException</div><div>    {</div><div>        final File file = File.createTempFile("ipc-", ".dat");</div><div>        file.deleteOnExit();</div><div><br></div><div>        return file;</div><div>    }</div><div><br></div><div>    private static void testIpc(final File file) throws Exception</div><div>    {</div><div>        writeZeros(file);</div><div><br></div><div>        final Reader reader = new Reader(file);</div><div>        final Thread readerThread = new Thread("READER")</div><div>        {</div><div>            @Override</div><div>            public void
 run()</div><div>            {</div><div>                try</div><div>                {</div><div>                    try</div><div>                    {</div><div>                        while (reader.read())</div><div>                        {</div><div>                            // busy spin</div><div>                        }</div><div>                    }</div><div>                    finally</div><div>                    {</div><div>                        reader.close();</div><div>                    }</div><div>                }</div><div>                catch (Exception ex)</div><div>                {</div><div>                    ex.printStackTrace();</div><div>   
             }</div><div>            }</div><div>        };</div><div>        readerThread.start();</div><div><br></div><div>        final Writer writer = new Writer(file);</div><div>        final Thread writerThread = new Thread("WRITER")</div><div>        {</div><div>            @Override</div><div>            public void run()</div><div>            {</div><div>                try</div><div>             
   {</div><div>                    try</div><div>                    {</div><div>                        while (writer.write() < 42)</div><div>                        {</div><div>                            // busy spin</div><div>                        }</div><div>                    }</div><div>                    finally</div><div> 
                   {</div><div>                        writer.close();</div><div>                    }</div><div>                }</div><div>                catch (Exception ex)</div><div>                {</div><div>                    ex.printStackTrace();</div><div>                }</div><div>            }</div><div>        };</div><div>       
 writerThread.start();</div><div><br></div><div>        writerThread.join();</div><div>        readerThread.join();</div><div><br></div><div>        if (reader.readCounter != writer.writerCounter)</div><div>        {</div><div>            throw new Error(</div><div>                "Reader exited prematurely: Reader.readCounter=" + reader.readCounter +</div><div>                ", Reader.writeCounter=" + reader.writeCounter +</div><div>                ", Writer.writeCounter=" + writer.writerCounter +</div><div>                ", iteration=" + iteration);</div><div>        }</div><div>    }</div><div><br></div><div>    private static void writeZeros(final File file) throws IOException</div><div>    {</div><div>        Files.write(file.toPath(), new byte[(int)IpcBase.TOTAL_LENGTH], StandardOpenOption.WRITE);</div><div>    }</div><div>}</div><div><br><div><blockquote style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Date: Mon, 8 Sep 2014 08:08:29 -0700<br>
From: Dmitry Vyazelenko <<a rel="nofollow" href="mailto:vyazelenko@yahoo.com" target="_blank">vyazelenko@yahoo.com</a>><br>
To: "<a rel="nofollow" href="mailto:concurrency-interest@cs.oswego.edu" target="_blank">concurrency-interest@cs.oswego.edu</a>"<br>
        <<a rel="nofollow" href="mailto:concurrency-interest@cs.oswego.edu" target="_blank">concurrency-interest@cs.oswego.edu</a>><br>
Subject: [concurrency-interest] Unsafe + DirectBuffer concurrency bug?<br>
Message-ID:<br>
        <<a rel="nofollow" href="mailto:1410188909.78423.YahooMailNeo@web162206.mail.bf1.yahoo.com" target="_blank">1410188909.78423.YahooMailNeo@web162206.mail.bf1.yahoo.com</a>><br>
Content-Type: text/plain; charset="utf-8"<br>
<br>
Hi all,<br>
<br>
Please find standalone test that points to potential Unsafe/DirectBuffer issue. The test uses memory mapped file (DirectBuffer) and Unsafe to communicate between reader and writer.<br>
The data layout within the buffer:<br>
<64 bytes padding><reader counter><64 bytes padding><writer counter><64 bytes padding><end of data flag><64 bytes padding><br>
Total number of bytes is 256.<br>
<br>
The Writer thread keeps incrementing writerCounter until it reaches value 42. At which point it also writes endOfDataFlag and closes the FileChannel.<br>
<br>
The Reader threads reads both readerCounter and writerCounter. If both have the same value and endOfDataFlag is set then it exits. Otherwise it increments readerCounter.<br>
<br>
Below is the complete test:<br>
<br>
package ipc_bug;<br>
<br>
import sun.misc.Unsafe;<br>
<br>
import java.io.File;<br>
import java.io.IOException;<br>
import java.io.RandomAccessFile;<br>
import java.lang.reflect.Field;<br>
import java.nio.MappedByteBuffer;<br>
import java.nio.channels.FileChannel;<br>
import java.nio.file.Files;<br>
import java.nio.file.StandardOpenOption;<br>
<br>
class IpcBase implements AutoCloseable {<br>
    private static final long PADDING = 64;<br>
    static final long READER_COUNTER_OFFSET = PADDING;<br>
    static final long WRITER_COUNTER_OFFSET = READER_COUNTER_OFFSET + PADDING;<br>
    static final long FLAG_OFFSET = WRITER_COUNTER_OFFSET + PADDING;<br>
    static final long TOTAL_LENGTH = FLAG_OFFSET + PADDING;<br>
<br>
    private static final Unsafe UNSAFE;<br>
    static {<br>
        try {<br>
            final Field field = Unsafe.class.getDeclaredField("theUnsafe");<br>
            field.setAccessible(true);<br>
            UNSAFE = (Unsafe) field.get(null);<br>
        } catch (final Exception ex) {<br>
            throw new RuntimeException(ex);<br>
        }<br>
    }<br>
<br>
    private final FileChannel channel;<br>
    private final MappedByteBuffer buffer;<br>
    private final long baseAddress;<br>
<br>
    public IpcBase(File file) throws IOException {<br>
        channel = new RandomAccessFile(file, "rw").getChannel();<br>
        buffer = channel.map(FileChannel.MapMode.READ_WRITE, 0, TOTAL_LENGTH);<br>
        baseAddress = ((sun.nio.ch.DirectBuffer) buffer).address();<br>
    }<br>
<br>
    final void putLongVolatile(long offset, long value) {<br>
        UNSAFE.putLongVolatile(null, baseAddress + offset, value);<br>
    }<br>
<br>
    final long getLongVolatile(long offset) {<br>
        return UNSAFE.getLongVolatile(null, baseAddress + offset);<br>
    }<br>
<br>
    @Override<br>
    public void close() throws Exception {<br>
        channel.close();<br>
    }<br>
}<br>
<br>
class Writer extends IpcBase {<br>
    public Writer(File file) throws IOException {<br>
        super(file);<br>
    }<br>
<br>
    @Override<br>
    public void close() throws Exception {<br>
        putLongVolatile(FLAG_OFFSET, 1L);<br>
        super.close();<br>
    }<br>
<br>
    long writerCounter;<br>
<br>
    public long write() {<br>
        writerCounter = getLongVolatile(WRITER_COUNTER_OFFSET);<br>
        putLongVolatile(WRITER_COUNTER_OFFSET, ++writerCounter);<br>
        return writerCounter;<br>
    }<br>
}<br>
<br>
class Reader extends IpcBase {<br>
    public Reader(File file) throws IOException {<br>
        super(file);<br>
    }<br>
<br>
    long readCounter;<br>
    long writeCounter;<br>
<br>
    public boolean read() {<br>
        readCounter = getLongVolatile(READER_COUNTER_OFFSET);<br>
        writeCounter = getLongVolatile(WRITER_COUNTER_OFFSET);<br>
        if (readCounter == writeCounter && isFlagSet()) {<br>
            return false;<br>
        } else if (readCounter < writeCounter) {<br>
            putLongVolatile(READER_COUNTER_OFFSET, ++readCounter);<br>
        }<br>
        return true;<br>
    }<br>
<br>
    private boolean isFlagSet() {<br>
        return 1L == getLongVolatile(FLAG_OFFSET);<br>
    }<br>
}<br>
<br>
public class IpcBugTest {<br>
    public static void main(String[] args) throws Exception {<br>
        File file = createFile();<br>
        for (int i = 0; i < 50_000; i++) {<br>
            testIPC(file);<br>
        }<br>
        System.out.println("NO BUG EXISTS! :-)");<br>
    }<br>
<br>
    private static File createFile() throws IOException {<br>
        File file = File.createTempFile("ipc-", ".dat");<br>
        file.deleteOnExit();<br>
        return file;<br>
    }<br>
<br>
    private static void testIPC(final File file) throws Exception {<br>
        writeZeros(file);<br>
<br>
        final Writer writer = new Writer(file);<br>
        Thread writerThread = new Thread("WRITER") {<br>
            @Override<br>
            public void run() {<br>
                try {<br>
                    try {<br>
                        while (writer.write() < 42) ;<br>
                    } finally {<br>
                        writer.close();<br>
                    }<br>
                } catch (Exception ex) {<br>
                    ex.printStackTrace();<br>
                }<br>
            }<br>
        };<br>
        writerThread.start();<br>
<br>
        final Reader reader = new Reader(file);<br>
        Thread readerThread = new Thread("READER") {<br>
            @Override<br>
            public void run() {<br>
                try {<br>
                    try {<br>
                        while (reader.read()) ;<br>
                    } finally {<br>
                        reader.close();<br>
                    }<br>
                } catch (Exception ex) {<br>
                    ex.printStackTrace();<br>
                }<br>
            }<br>
        };<br>
        readerThread.start();<br>
<br>
        writerThread.join();<br>
        readerThread.join();<br>
<br>
        if (reader.readCounter != writer.writerCounter) {<br>
            throw new Error("Reader exited prematurely: Reader.readCounter=" + reader.readCounter<br>
                    + ", Reader.writeCounter=" + reader.writeCounter<br>
                    + ", Writer.writeCounter=" + writer.writerCounter);<br>
        }<br>
    }<br>
<br>
    private static void writeZeros(File file) throws IOException {<br>
        Files.write(file.toPath(), new byte[(int) IpcBase.TOTAL_LENGTH], StandardOpenOption.WRITE);<br>
    }<br>
}<br>
<br>
This test fails on my Windows 7 machine using JDK 7u67 and JDK 8u20 (older Intel i5). It also fails on Intel i7 IvyBridge machine that is running Ubuntu 14.04 on same JDKs. The failure is random, i.e. I've observed it fail on first iteration, 600, after 10K iterations etc. but it fails eventually on each run I tried.<br>
<br>
The failure message shown is:<br>
    java.lang.Error: Reader exited prematurely: Reader.readCounter=0, Reader.writeCounter=0, Writer.writeCounter=42<br>
<br>
In this case the reader thread observed both counters to have the same value 0 and endOfDataFlag is set. However writer thread writes endOfDataFlag after final writerCounter (i.e. 42) is being written.<br>
<br>
I hope the bug is in my code. So please tell me how wrong I am! ;)<br>
<br>
Best regards,<br>
Dmitry Vyazelenko<br></blockquote></div></div></div></div></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" target="_blank">http://cs.oswego.edu/mailman/listinfo/concurrency-interest</a><br><br><br></div>  </div> </div>  </div> </div></div></div></div></blockquote></div><br></div></div>
<br>_______________________________________________<br>
Concurrency-interest mailing list<br>
<a href="mailto:Concurrency-interest@cs.oswego.edu">Concurrency-interest@cs.oswego.edu</a><br>
<a href="http://cs.oswego.edu/mailman/listinfo/concurrency-interest" target="_blank">http://cs.oswego.edu/mailman/listinfo/concurrency-interest</a><br>
<br></blockquote></div><br></div>