Re: Externalizable in cache

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Externalizable in cache

Vladimir Ozerov
Myron,
Thank you for pointing this. it looks like we violate stream contract
because regular Java ObjectOutputStream will return -1 in this case.
Forwarding to dev list.

Igniters,
As Myron mentioned, out implementation of ObjectInput returns 0 when
performing ObjectInput.read(byte[]) and there is no more array data
available. To the contrast, Java's ObjectInputStream returns -1 in this
case.
Looks like we should fix that.

Thoughts?

Vladimir.

On Mon, Feb 15, 2016 at 10:32 PM, Myron Chelyada <[hidden email]>
wrote:

> Hello Alexey,
>
> This is actually not real code but just extracted part for test. That's
> why it looks incorrect.
> And I've already captured infinite loop because of 0 being returned. And
> imagine use case when I need to read until end of stream. So question is
> why 0 is returned but not -1 (according to API).
>
>
> 2016-02-15 21:10 GMT+02:00 Dmitriy Setrakyan <[hidden email]>:
>
>> Alexey, is this something that Ignite could prevent automatically?
>>
>> On Mon, Feb 15, 2016 at 10:17 AM, Alexey Goncharuk <
>> [hidden email]> wrote:
>>
>>> A little correction: in this particular case inputStream does return 0
>>> which leads to an infinite loop, however, generally this may be not the
>>> case, so the implementation should not read beyond object boundary anyways.
>>>
>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: Externalizable in cache

Alexey Goncharuk
Agree that this should be fixed. However, from the end-user perspective
this should never be an issue because at some point in future Ignite might
need to append some extra bytes to the Externalizable object layout, thus
reading beyond that limit will break the unmarshalling process.

If, by the time we invoke readExternal, we know the size of the written
externalizable object then we can create a thin wrapper around the existing
input stream which will provide the desired behavior. However, this will
come at a cost of generating more garbage, so I would just fix the return
value for the input stream.
Reply | Threaded
Open this post in threaded view
|

Re: Externalizable in cache

Myron Chelyada
In reply to this post by Vladimir Ozerov
Thanks,

And sorry for a little bit confusing test I provided.

2016-02-16 13:46 GMT+02:00 Vladimir Ozerov <[hidden email]>:

> Myron,
> Thank you for pointing this. it looks like we violate stream contract
> because regular Java ObjectOutputStream will return -1 in this case.
> Forwarding to dev list.
>
> Igniters,
> As Myron mentioned, out implementation of ObjectInput returns 0 when
> performing ObjectInput.read(byte[]) and there is no more array data
> available. To the contrast, Java's ObjectInputStream returns -1 in this
> case.
> Looks like we should fix that.
>
> Thoughts?
>
> Vladimir.
>
> On Mon, Feb 15, 2016 at 10:32 PM, Myron Chelyada <[hidden email]
> > wrote:
>
>> Hello Alexey,
>>
>> This is actually not real code but just extracted part for test. That's
>> why it looks incorrect.
>> And I've already captured infinite loop because of 0 being returned. And
>> imagine use case when I need to read until end of stream. So question is
>> why 0 is returned but not -1 (according to API).
>>
>>
>> 2016-02-15 21:10 GMT+02:00 Dmitriy Setrakyan <[hidden email]>:
>>
>>> Alexey, is this something that Ignite could prevent automatically?
>>>
>>> On Mon, Feb 15, 2016 at 10:17 AM, Alexey Goncharuk <
>>> [hidden email]> wrote:
>>>
>>>> A little correction: in this particular case inputStream does return 0
>>>> which leads to an infinite loop, however, generally this may be not the
>>>> case, so the implementation should not read beyond object boundary anyways.
>>>>
>>>
>>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: Externalizable in cache

Vladimir Ozerov
In reply to this post by Alexey Goncharuk
Alex,

In ObjectOutputStream if you read byte array and still have some other data
in the stream, it will return you -1. So as I understand, from user
perspective it is normal to wait for -1. What do you think?

Vladimir.

On Tue, Feb 16, 2016 at 2:54 PM, Alexey Goncharuk <
[hidden email]> wrote:

> Agree that this should be fixed. However, from the end-user perspective
> this should never be an issue because at some point in future Ignite might
> need to append some extra bytes to the Externalizable object layout, thus
> reading beyond that limit will break the unmarshalling process.
>
> If, by the time we invoke readExternal, we know the size of the written
> externalizable object then we can create a thin wrapper around the existing
> input stream which will provide the desired behavior. However, this will
> come at a cost of generating more garbage, so I would just fix the return
> value for the input stream.
>