Skip to content
This repository has been archived by the owner on Nov 22, 2023. It is now read-only.

Possible "Out of Memory" danger for opcode NEWBUFFER and PUSHDATA #369

Closed
Qiao-Jin opened this issue Sep 25, 2020 · 7 comments
Closed

Possible "Out of Memory" danger for opcode NEWBUFFER and PUSHDATA #369

Qiao-Jin opened this issue Sep 25, 2020 · 7 comments

Comments

@Qiao-Jin
Copy link

Qiao-Jin commented Sep 25, 2020

Currently MaxStackSize = 2048 and MaxItemSize = 1M, which means that we can at most push 2GB's data into stack. 2GB's size might seems not so much for nowaday computer's RAM. However, this piece of data is stored in a List which is stored in ram continuously. Seeking a continuous piece of 2GB-sized RAM might give rise to Out of Memory exception for computers whose RAM is more than 2GB.

My computer is with 16GB's RAM. Tests shows that an Out of Memory exception will be thrown after about 1500 1MB-sized new buffers are created & pushed into stack.

This will give rise to a serious problem: result of transaction execution will differ on different nodes, which might lead to other problems and eventually consensus crash.

@shargon
Copy link
Member

shargon commented Sep 25, 2020

Could you provide the script?

@Qiao-Jin
Copy link
Author

Qiao-Jin commented Sep 25, 2020

Could you provide the script?

int times = 2000;
using (var engine = new ExecutionEngine())
using (var script = new ScriptBuilder())
{
    byte[] args = new BigInteger(1024 * 1024).ToByteArray().AsEnumerable().Append<byte>(0).ToArray();
    for (int i = 0; i < times; i++)
    {
        script.Emit(OpCode.PUSHINT32, args);
        script.Emit(OpCode.NEWBUFFER);
    }
    engine.LoadScript(script.ToArray());
    engine.Execute();
}

@shargon
Copy link
Member

shargon commented Sep 25, 2020

It doesn't produce the same behavior in my computer...

image

Only if I inspect the ResultStack it will be frozen

image

@shargon
Copy link
Member

shargon commented Sep 25, 2020

But yes, we should reduce the limits or use a different system, I proposed time ago #245 in order to forget this limits.

@Qiao-Jin
Copy link
Author

Qiao-Jin commented Sep 25, 2020

It doesn't produce the same behavior in my computer...

Yeah, that's an example where result of transaction execution will differ on different nodes

@erikzhang
Copy link
Member

Seeking a continuous piece of 2GB-sized RAM might give rise to Out of Memory exception

Why do you need a continuous piece of 2GB-sized RAM?

@erikzhang
Copy link
Member

erikzhang commented Sep 25, 2020

static void Main()
{
    List<byte[]> list = new List<byte[]>();
    for (int i = 0; i < 10240; i++)
        list.Add(new byte[1024 * 1024]);
}

I don't think it's a problem. This code works fine.

And, OS manages the memory in paging and does not need contiguous memory at all. I think what you have encountered should be other problems.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants