Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some type of variables are not safely copied by Script.Clone. #336

Open
matszpk opened this issue May 24, 2021 · 3 comments
Open

Some type of variables are not safely copied by Script.Clone. #336

matszpk opened this issue May 24, 2021 · 3 comments

Comments

@matszpk
Copy link

matszpk commented May 24, 2021

My experiments with Script.Clone method give unexpected results. After cloning Compiled object, the global map and arrays values has been copied to origin Compiled object. Your code doesn't make new copy array values and map values, just simply copy globals array and your code of Clone doesn't make safe copy of Compiled object. Global mutable maps and arrays should be copied deeply to make safe-concurrent copy of state.

@misiek08
Copy link
Contributor

@matszpk can you provide simplest example or an test to show this behavior? This will be close to fixing so even patch is welcome :)

@ipsusila
Copy link
Contributor

ipsusila commented Sep 22, 2022

First of all, thank you for the great works and contributions in Tengo.
I just start experimenting with Tengo and found this issue. I think the following example script shows what @matszpk means.

// A script with two global variables assigned by host
// 1. `round` an integer type
// 2. `data` a map type

// 1. overwrite primitive type, safe
round = 1000

last_key := ""
for key, val in data {
    last_key = key
}
// 2. delete an entry from data (map type) is not safe in `cloned` bytecode
// the value in original compiled bytecode is being modified
delete(data, last_key)

I verified the behaviour, and propose a patch at ipsusila@14bfd45.

@geseq
Copy link
Collaborator

geseq commented Sep 22, 2022

@ipsusila can you create a PR please

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

No branches or pull requests

4 participants