andrewbissada / kryo

Automatically exported from code.google.com/p/kryo
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

readClassAndObject throws KryoException: Buffer underflow. #128

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
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);
    }

}

Original issue reported on code.google.com by Kolor...@gmail.com on 22 Aug 2013 at 7:36

Attachments:

GoogleCodeExporter commented 9 years ago
if change thread count to 1, is ok
so, I think this issue based on multi-thread situation

Original comment by Kolor...@gmail.com on 22 Aug 2013 at 7:47

GoogleCodeExporter commented 9 years ago
https://code.google.com/p/kryo/#Threading
:)

Original comment by nathan.s...@gmail.com on 22 Aug 2013 at 6:49

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
did you see my code?
In my code, each thread has its own Kryo, Input, and Output instance
but also throws this exception

Original comment by Kolor...@gmail.com on 23 Aug 2013 at 2:53

GoogleCodeExporter commented 9 years ago
Ah, sorry, didn't see the TLS in KryoSerializer. 

Original comment by nathan.s...@gmail.com on 23 Aug 2013 at 11:08

GoogleCodeExporter commented 9 years ago
@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

Original comment by romixlev on 23 Aug 2013 at 12:49

GoogleCodeExporter commented 9 years ago
@Nate: I was not sure you'd get the previous comment from me, so I added you to 
CC. 

Original comment by romixlev on 23 Aug 2013 at 12:50

GoogleCodeExporter commented 9 years ago
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.

Original comment by nathan.s...@gmail.com on 23 Aug 2013 at 1:55

GoogleCodeExporter commented 9 years ago
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.

Original comment by romixlev on 23 Aug 2013 at 2:14

GoogleCodeExporter commented 9 years ago
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/kr
yo/kryo/) 

I would do these updates myself, but it looks like I don't have permissions to 
edit the page.

-Leo

Original comment by romixlev on 24 Aug 2013 at 2:56

GoogleCodeExporter commented 9 years ago
Thanks for your attention

Original comment by Kolor...@gmail.com on 2 Sep 2013 at 7:49

GoogleCodeExporter commented 9 years ago

Original comment by romixlev on 2 Oct 2013 at 3:07

GoogleCodeExporter commented 9 years ago
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);

Original comment by guram.sa...@gmail.com on 15 Oct 2014 at 7:34

GoogleCodeExporter commented 9 years ago
The project has moved to github. Please report your problems there or on the 
kryo mailing list.

Original comment by romixlev on 15 Oct 2014 at 7:37