readClassAndObject throws KryoException: Buffer underflow.

[ghost]

From Kolor...@gmail.com on August 22, 2013 09:36:08

this exception happens in my kryo performance test
invoke readClassAndObject 1000000 times in 100 thread

What steps will reproduce the problem?
1.multi-thread invoke readClassAndObject
2.each thread with kryo,input and output sington instance in thread local
3.each thread get kryo,input and output instance from thread local,and invoke kryo.readClassAndObject

What is the expected output? What do you see instead?
com.esotericsoftware.kryo.KryoException: Buffer underflow.
at com.esotericsoftware.kryo.io.Input.require(Input.java:156)
at com.esotericsoftware.kryo.io.Input.readInt(Input.java:337)
at com.esotericsoftware.kryo.util.DefaultClassResolver.readClass(DefaultClassResolver.java:109)
at com.esotericsoftware.kryo.Kryo.readClass(Kryo.java:610)
at com.esotericsoftware.kryo.Kryo.readClassAndObject(Kryo.java:721)
at com.esotericsoftware.kryo.serializers.MapSerializer.read(MapSerializer.java:134)
at com.esotericsoftware.kryo.serializers.MapSerializer.read(MapSerializer.java:17)
at com.esotericsoftware.kryo.Kryo.readClassAndObject(Kryo.java:729)
at push.serializer.KryoSerializer.deserialize(KryoSerializer.java:40)
at push.KryoTest.decode(KryoTest.java:68)
at push.KryoTest.access$0(KryoTest.java:67)
at push.KryoTest$1.run(KryoTest.java:44)
at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
at java.lang.Thread.run(Thread.java:662)

What version of the Kryo are you using?
2.21

Please provide any additional information below.
/**

• 
  /
  package push;
  
  import java.util.HashMap;
  import java.util.UUID;
  import java.util.concurrent.ExecutorService;
  import java.util.concurrent.Executors;
  import java.util.concurrent.TimeUnit;
  import java.util.concurrent.atomic.AtomicInteger;
  
  import push.serializer.KryoSerializer;
  
  /*
• @author kolor
• 
  */
  public class KryoTest {
  
  private static final KryoSerializer serializer = new KryoSerializer();
  
  public static void main(String[] args) throws Exception {
  final HashMap<String, Object> map = new HashMap<String, Object>();
  for (int i = 0; i < 10; i++) {
  map.put(UUID.randomUUID().toString() + i, UUID.randomUUID().toString());
  }
  //
  
  final byte[] bytes = encode(map);
  System.out.println(bytes.length);
  System.out.println(decode(bytes));
  final AtomicInteger seq = new AtomicInteger();
  ExecutorService executor = Executors.newFixedThreadPool(100);
  long bt = System.currentTimeMillis();
  //
  for (int i = 0; i < 1000000; i++) {
  executor.execute(new Runnable() {
  @OverRide
  public void run() {
  try {
  //
  // encode(map);
  decode(bytes);
  } catch (Exception e) {
  if (seq.incrementAndGet() == 1) {
  e.printStackTrace();
  }
  }
  }
  });
  }
  executor.shutdown();
  executor.awaitTermination(10000, TimeUnit.DAYS);
  
  long et = System.currentTimeMillis();
  
  System.out.println("cost " + (et - bt) + " fails:" + seq.get());
  }
  
  private static byte[] encode(HashMap<String, Object> map) {
  return serializer.serialize(map);
  }
  
  private static HashMap<String, Object> decode(byte[] data) throws Exception {
  return (HashMap<String, Object>) serializer.deserialize(data);
  }
  
  }
  

Attachment: Serializer.java KryoSerializer.java KryoTest.java

Original issue: http://code.google.com/p/kryo/issues/detail?id=128

[ghost]

From Kolor...@gmail.com on August 22, 2013 00:47:35

if change thread count to 1, is ok
so, I think this issue based on multi-thread situation

[ghost]

From nathan.s...@gmail.com on August 22, 2013 11:49:19

https://code.google.com/p/kryo/#Threading
:)

Status: Invalid

[ghost]

From Kolor...@gmail.com on August 22, 2013 19:53:05

did you see my code?
In my code, each thread has its own Kryo, Input, and Output instance
but also throws this exception

[ghost]

From nathan.s...@gmail.com on August 23, 2013 04:08:58

Ah, sorry, didn't see the TLS in KryoSerializer.

Status: Accepted

[ghost]

From romixlev on August 23, 2013 05:49:16

@nate:
Actually, this is a valid bug report and there is a bug in Input.readAscii(). It manipulates its buffer in-place, which may lead to problems in multi-threaded applications when the same byte buffer is shared by many Input objects.

I see two way to handle this situation:

1. Fix the Input.readAscii() method and allocate a temporary byte buffer, instead of doing in-place manipulations with a byte buffer of the Input object. This solves the problem but most likely affects performance.

2. Provide a guideline in the documentation that the same byte buffer should not be used by multiple Input objects simultaneously in multi-threaded environments. In this case, we can leave the implementation as is.

@nate: What do you think? Which of these approaches we should take?

-Leo

[ghost]

From romixlev on August 23, 2013 05:50:42

@nate: I was not sure you'd get the previous comment from me, so I added you to CC.

Cc: nathan.s...@gmail.com

[ghost]

From nathan.s...@gmail.com on August 23, 2013 06:55:18

All comments on all issues get emailed to me, but the CC is fine too. Much thanks for pinpointing the problem! For others, it is here:
https://code.google.com/p/kryo/source/browse/trunk/src/com/esotericsoftware/kryo/io/Input.java#577
The last character in a 7-bit ASCII sequence has the 8th bit set to denote the end. Passing this to "new String" obviously would corrupt that character. The ASCII path is a fast path, so copying the buffer here would be suboptimal. Is it a real use case to deserialize the same byte[] concurrently? It comes up in tests and I suppose it could come up in real code, but it seems rare. I'm not sure it is worth changing our nice fast path to fix. My vote is to just update the documentation. We can continue discussion, but for now I've updated the webpage and javadocs.

[ghost]

From romixlev on August 23, 2013 07:14:29

I second your vote, Nate. Using the same byte[] concurrently is a bit artificial, IMHO. After all, the client can create copies of this byte array if required.

[ghost]

From romixlev on August 24, 2013 07:56:15

BTW, Nate, it would be nice to add this information about issues with multi-threaded deserialization into the corresponding section on the main page of the project. It it sort of stated there, but IMHO it is not very clear for a new user. May be talking about Input streams backed by byte arrays would be more clear. And may be we should provide a code example showing how it may happen, e.g.:
byte[] buf = new byte[1024];
// read buf from a file
Input in1 = new Input(buf);
Input in2 = new Input(buf);

Processing in1 and in2 concurrently may lead to problems. Don't do it. If you need to do something like this, better create a dedicated copy of "buf" for each Input stream.

And if you'd update the page anyways, it would be also very nice to add the following information there:

• The version of the latest snapshot
• Information where the snapshots can be found:
  Latest Kryo snapshots are available in the sonatype snapshots repo:
  https://oss.sonatype.org/content/repositories/snapshots/
  (https://oss.sonatype.org/content/repositories/snapshots/com/esotericsoftware/kryo/kryo/)
  
  I would do these updates myself, but it looks like I don't have permissions to edit the page.
  
  -Leo

[ghost]

From Kolor...@gmail.com on September 02, 2013 00:49:10

Thanks for your attention

[ghost]

From romixlev on October 02, 2013 08:07:58

(No comment was entered for this change.)

Status: WontFix

[mirion]

I know that this bug is closed with wontfix but I think that the resolution is wrong and the issue has to be addressed thoroughly.

A brief explanation: I'm using Kryo to share informations between applications running into several j2ee servers. This data is routed through the sessions, therefore it may be requested by two threads in the same time. If I'm following your advice, I have to systematically duplicate the buffer in order to safely read it. Of course, this comes with a much bigger penalty than duplicating just the culprit, the encoded ascii data.

Your software is great but on the other hand, I really don't understand why are you taking this approach: a sort of global memory variables that can't be shared (the data buffers). I understand that the Kryo instances themselves are not multi-threading enabled, this is not a problem, but the data itself? Since this happens with byte arrays, already loaded into the memory, it is like having a house where if somebody is looking though a window, nobody else can do it in the same time. This is a dramatic limitation and can't be addressed by systematically breaking the wall to open another window...

Your software is designed for speed, but first of all it should be designed for reliability and the optimization comes afterwards.

I understand that these design choices have been taken long time ago, but maybe now, when the software is mature enough and in a good shape it is possible to fix some of the bugs/limitations.

Thanks

[NathanSweet]

For people not accessing the byte[] from multiple threads (which I think is extremely common), the readAscii fast path is nice and fast. It would be great to find an alternative that is just as fast without adding to the serialized size.

For now you could extend Output and implement your own writeString and extend Input and implement your own readString. You can either send an extra byte per string so the "end of ASCII" bit isn't in the string data, or you can copy the string bytes, mask the last one, then create the string.

[mirion]

You are really fast... Thank you

I understand that there are quick workarounds, but do you really think that turning this into the standard behavior is so expensive? I doubt it. What is the cost of a byte compared to the predictable and reliable behavior?

In my opinion, this inability to read data buffers through multiple threads is a limitation that compared to the minor speed bump becomes really big an in the end a pain for the developers.

In the same time, multi-threading is common into web development therefore the problem is not so infrequent. Probably most of those getting it just avoided the issue through a buffer duplication, or another trick.

Please consider a general resolution of this kind of root problem. It will only make your software better.

Thanks

[savinov]

I have this very exception in single thread program with one input/output instance.

Code is very simple:

Kryo kryo = new Kryo();
List objects = ...
Path storagePath = ...
Output output = new Output(Files.newOutputStream(storagePath));
kryo.writeClassAndObject(output, objects);

Input input = new Input(Files.newInputStream(storagePath));
List data = (List) kryo.readClassAndObject(input);

[romix]

You probably need to close your output stream first before you read from the file, so that it flushes.

[serverperformance]

Hi, you didn’t close the Output before starting reading…..

See https://github.com/EsotericSoftware/kryo#quickstart

Cheers,

Tumi

De: Guram Savinov [mailto:notifications@github.com]
Enviado el: miércoles, 15 de octubre de 2014 9:38
Para: EsotericSoftware/kryo
Asunto: Re: [kryo] readClassAndObject throws KryoException: Buffer underflow. (#128)

I have this very exception in single thread program with one input/output instance.

Code is very simple:

Kryo kryo = new Kryo();
List objects = ...
Path storagePath = ...
Output output = new Output(Files.newOutputStream(storagePath));
kryo.writeClassAndObject(output, objects);

Input input = new Input(Files.newInputStream(storagePath));
List data = (List) kryo.readClassAndObject(input);

—
Reply to this email directly or view it on GitHub #128 (comment) . https://github.com/notifications/beacon/6210680__eyJzY29wZSI6Ik5ld3NpZXM6QmVhY29uIiwiZXhwaXJlcyI6MTcyODk3Nzg3NywiZGF0YSI6eyJpZCI6MjAwNTY5NTJ9fQ==--12502b959b803c868852088735281b6f3b28132e.gif

[savinov]

I close output before reading, more detailed code looks like this:

import com.esotericsoftware.kryo.Kryo;
import com.esotericsoftware.kryo.io.Input;
import com.esotericsoftware.kryo.io.Output;
import org.apache.commons.io.IOUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;

/**
 * @author Guram Savinov
 */
public class KryoStorage {

    private static final Kryo KRYO = new Kryo();
    private static final Logger LOGGER = LoggerFactory.getLogger(KryoStorage.class);

    private Path storagePath;

    public KryoStorage(Path storagePath) {
        this.storagePath = storagePath;
    }

    public void saveContext(List objects) {
        Output output = null;
        try {
            output = new Output(Files.newOutputStream(storagePath));
            KRYO.writeClassAndObject(output, objects);
        } catch (IOException e) {
            LOGGER.warn("I/O error  while write to the storage file", e);
        } finally {
            IOUtils.closeQuietly(output);
        }
    }

    public List loadContext() {
        List result = new ArrayList();
        if (Files.isReadable(storagePath)) {
            Input input = null;
            try {
                input = new Input(Files.newInputStream(storagePath));
                result = (List) KRYO.readClassAndObject(input);
            } catch (IOException e) {
                LOGGER.warn("I/O error while reading from the storage file", e);
            } finally {
                IOUtils.closeQuietly(input);
            }
        }

        try {
            Files.deleteIfExists(storagePath);
        } catch (IOException e) {
            LOGGER.warn("I/O error while delete storage file", e);
        }

        return result;
    }
}

There is a pause about few minutes between saveContext() and loadContext(), moreover saveContext() and loadContext() executes from different JVM: after saveContext is done, first JVM stops, then starting second JVM process and execute loadContext().
saveContext() have stored data without any problems, but now I have a file which cause KryoException: Buffer underflow in loadContext() on every attempt to read data from it.
File contain single java.util.ArrayList which contain one java.util.Map<String, MyClass> with about 1500000 kay/value pairs and has size about 50Mb.
Unforunately I can't give you this file for testing, because it has secret data.
Earlier this code works without any problems with same data this size.
Kryo version was 2.24.0 given from the maven repo, 3.0.0 release cause this exception also with reading my data from file.
Maybe my stacktrace will be useful:

Caused by: com.esotericsoftware.kryo.KryoException: Buffer underflow.
        at com.esotericsoftware.kryo.io.Input.require(Input.java:181) ~[kryo-2.24.0.jar:na]
        at com.esotericsoftware.kryo.io.Input.readVarInt(Input.java:355) ~[kryo-2.24.0.jar:na]
        at com.esotericsoftware.kryo.util.DefaultClassResolver.readClass(DefaultClassResolver.java:109) ~[kryo-2.24.0.jar:na]
        at com.esotericsoftware.kryo.Kryo.readClass(Kryo.java:641) ~[kryo-2.24.0.jar:na]
        at com.esotericsoftware.kryo.Kryo.readClassAndObject(Kryo.java:752) ~[kryo-2.24.0.jar:na]
        at com.esotericsoftware.kryo.serializers.MapSerializer.read(MapSerializer.java:135) ~[kryo-2.24.0.jar:na]
        at com.esotericsoftware.kryo.serializers.MapSerializer.read(MapSerializer.java:21) ~[kryo-2.24.0.jar:na]
        at com.esotericsoftware.kryo.Kryo.readClassAndObject(Kryo.java:761) ~[kryo-2.24.0.jar:na]
        at com.esotericsoftware.kryo.serializers.CollectionSerializer.read(CollectionSerializer.java:116) ~[kryo-2.24.0.jar:na]
        at com.esotericsoftware.kryo.serializers.CollectionSerializer.read(CollectionSerializer.java:22) ~[kryo-2.24.0.jar:na]
        at com.esotericsoftware.kryo.Kryo.readClassAndObject(Kryo.java:761) ~[kryo-2.24.0.jar:na]

[savinov]

Seems like problem is EOF detection, check out the topic on StackOverflow

[romix]

Could you provide a complete unit test? I quickly tested it on my machine and it works for me. I just replaced loggers and Apache utils by simpler code, which does not use any external dependencies. And it seems to work without any problems.

[savinov]

Unit test doesn't reproduce this bug, I have file that Kryo succesfully serialized with saveContext() that Kryo can't read via loadContext().
I'm not alone who found this problem, see StackOverflow posts and comments at the top of this discussion:

did you see my code?
In my code, each thread has its own Kryo, Input, and Output instance
but also throws this exception
#128 (comment)

[romix]

But you can try to reproduce this bug with a unit test, or? Add something to make it look more like your real test-case. Add more complexity, etc, but try to make it trigger this bug. Otherwise it is a bit difficult to fix it.

BTW, there could be totally different root causes for buffer underflow exceptions. That's why it is important to have something reproducible.

[savinov]

I think problem is the number of Map records, maybe Kryo write wrong value to the file and try to get more then file contain.
This unit-test reproduce a problem (it use KryoStorage class, see in my previous comment). When number of map records is 100 - it's Ok, 200 - Ok too, but if set it to 500 - get assertion error: written 500, but read 499 or 498. Why?

import org.junit.Test;

import java.nio.file.Paths;
import java.util.*;

/**
 * @author Guram Savinov
 */
public class KryoTest {

    private static final int NUMBER_OF_MAP_RECORDS = 500;

    @Test
    public void readStorage() {
        for (int i = 0; i < 10; i++) {
            writeAndRead();
        }
    }

    private void writeAndRead() {
        KryoStorage kryoStorage = new KryoStorage(Paths.get("/tmp/test.kryo"));
        Map<String, MyClass> dataMap = new HashMap<>();
        Random random = new Random();
        for (int i = 0; i < NUMBER_OF_MAP_RECORDS; i++) {
            dataMap.put(UUID.randomUUID().toString().substring(0, 5), new MyClass(random.nextInt(128), random.nextInt(128),
                    random.nextInt(128), UUID.randomUUID().toString()));
        }
        List dataToSave = new ArrayList();
        dataToSave.add(dataMap);
        kryoStorage.saveContext(dataToSave);
        List readData = kryoStorage.loadContext();
        org.junit.Assert.assertEquals(NUMBER_OF_MAP_RECORDS, ((Map) readData.get(0)).size());
    }

    public static class MyClass {
        private int num1;
        private int num2;
        private int num3;
        private String str1;

        public MyClass() {
        }

        public MyClass(int num1, int num2, int num3, String str1) {
            this.num1 = num1;
            this.num2 = num2;
            this.num3 = num3;
            this.str1 = str1;
        }

        public int getNum1() {
            return num1;
        }

        public void setNum1(int num1) {
            this.num1 = num1;
        }

        public int getNum2() {
            return num2;
        }

        public void setNum2(int num2) {
            this.num2 = num2;
        }

        public int getNum3() {
            return num3;
        }

        public void setNum3(int num3) {
            this.num3 = num3;
        }

        public String getStr1() {
            return str1;
        }

        public void setStr1(String str1) {
            this.str1 = str1;
        }
    }
}

[savinov]

More interesting: remove for loop and set NUMBER_OF_MAP_RECORDS = 1000000

import org.junit.Test;

import java.nio.file.Paths;
import java.util.*;

/**
 * @author Guram Savinov
 */
public class KryoTest {

    private static final int NUMBER_OF_MAP_RECORDS = 1000000;


    @Test
    public void writeAndRead() {
        KryoStorage kryoStorage = new KryoStorage(Paths.get("/tmp/test.kryo"));
        Map<String, MyClass> dataMap = new HashMap<>();
        Random random = new Random();
        for (int i = 0; i < NUMBER_OF_MAP_RECORDS; i++) {
            dataMap.put(UUID.randomUUID().toString().substring(0, 5), new MyClass(random.nextInt(128), random.nextInt(128),
                    random.nextInt(128), UUID.randomUUID().toString()));
        }
        List dataToSave = new ArrayList();
        dataToSave.add(dataMap);
        kryoStorage.saveContext(dataToSave);
        List readData = kryoStorage.loadContext();
        org.junit.Assert.assertEquals(NUMBER_OF_MAP_RECORDS, ((Map) readData.get(0)).size());
    }

    public static class MyClass {
        private int num1;
        private int num2;
        private int num3;
        private String str1;

        public MyClass() {
        }

        public MyClass(int num1, int num2, int num3, String str1) {
            this.num1 = num1;
            this.num2 = num2;
            this.num3 = num3;
            this.str1 = str1;
        }

        public int getNum1() {
            return num1;
        }

        public void setNum1(int num1) {
            this.num1 = num1;
        }

        public int getNum2() {
            return num2;
        }

        public void setNum2(int num2) {
            this.num2 = num2;
        }

        public int getNum3() {
            return num3;
        }

        public void setNum3(int num3) {
            this.num3 = num3;
        }

        public String getStr1() {
            return str1;
        }

        public void setStr1(String str1) {
            this.str1 = str1;
        }
    }
}

Expected :1000000
Actual :645149

Where is other Map 354851 records?

[romix]

Very interesting. I'll look into this.

[romix]

Just tried it on my side. I can reproduce it. Looking into it.

[romix]

OK. There is a bug in your test:
You use this as a key: UUID.randomUUID().toString().substring(0, 5)
But if you call it enough times, it starts to generate the same keys. So, if you replace it by:
UUID.randomUUID().toString(), then it works properly.

You need to try harder if you want to reproduce your bug ;-)