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

fix(gnovm): correctly update refs count when self-assigning #960

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/gno.land/r/demo/multitxtest/gno.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module gno.land/r/demo/multitxtest
62 changes: 62 additions & 0 deletions examples/gno.land/r/demo/multitxtest/multitx_testing.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package multitxtest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these tests be contained in the examples subdirectory, instead of gnovm, since these are .go files after all?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should keep them in the examples folder, alongside the contract implementation.

I started this in #979, but paused to prioritize #1016, which will also provide a go API for advanced gno tests.

In the future, we plan to remove those _test.go files completely in favor of a gno-centric multi-step approach defined in #934.

By the way, I suggest moving the entire realm to the r/demo/x/ subfolder for experiments and things we don't want people to interact with.

Copy link
Member

@moul moul Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the _test.go file is loading the local .gno file, my suggestion is to create an official folder for these flows, located at gnovm/tests/flows/. We can place multitx.go in this folder as well as the other file, issue939/flow_test.go. This way, the examples folder can remain dedicated to showcasing real examples.


import (
"os"
"path/filepath"
"testing"

"github.com/gnolang/gno/gnovm/pkg/gnolang"
"github.com/gnolang/gno/gnovm/stdlibs"
"github.com/gnolang/gno/gnovm/tests"
"github.com/gnolang/gno/tm2/pkg/std"
)

type TxDefinition struct {
Pkg *std.MemPackage
Entrypoint string
OrigSend std.Coins
ExpectPanic bool
}

func RunTxs(t *testing.T, txs []TxDefinition) {
var (
mode = tests.ImportModeStdlibsOnly
rootDir = filepath.Join("..", "..", "..", "..", "..")
stdin = os.Stdin
stdout = os.Stdout
stderr = os.Stderr
store = tests.TestStore(rootDir, "", stdin, stdout, stderr, mode)
)
store.SetStrictGo2GnoMapping(true) // natives must be registered
gnolang.DisableDebug() // until main call
m := tests.TestMachine(store, stdout, "main")
for i, tx := range txs {
ctx := m.Context.(stdlibs.ExecContext)
ctx.OrigSend = tx.OrigSend
if i > 0 {
ctx.Height++
ctx.Timestamp++
}
m.Context = ctx
memPkg := tx.Pkg
m.RunMemPackage(memPkg, true)
store.ClearCache()
m.PreprocessAllFilesAndSaveBlockNodes()
pv2 := store.GetPackage(tx.Pkg.Path, false)
if i == 0 {
m.SetActivePackage(pv2)
}
gnolang.EnableDebug()
defer func() {
r := recover()
if tx.ExpectPanic && r == nil {
t.Fatalf("expected panic in "+tx.Entrypoint+"\n%s\n", r, m.String())
}
if !tx.ExpectPanic && r != nil {
t.Fatalf(tx.Entrypoint+" panic: %v\n%s\n", r, m.String())
}
}()
m.RunStatement(gnolang.S(gnolang.Call(gnolang.X(tx.Entrypoint))))
m.CheckEmpty()
}
}
77 changes: 77 additions & 0 deletions examples/gno.land/r/demo/multitxtest/slice_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package multitxtest

import (
"testing"

"github.com/gnolang/gno/tm2/pkg/std"
)

func TestSlicePopPush(t *testing.T) {
pkgName := "main"
pkgPath := "gno.land/r/demo/slicetest"
RunTxs(t, []TxDefinition{
{
Pkg: &std.MemPackage{
Name: pkgName,
Path: pkgPath,
Files: []*std.MemFile{
{
Name: "main1.gno",
Body: `
package main
import "gno.land/r/demo/multitxtest"
import "std"
func main1() {
multitxtest.Pop()
}
`,
},
},
},
Entrypoint: "main1",
},
{
Pkg: &std.MemPackage{
Name: pkgName,
Path: pkgPath,
Files: []*std.MemFile{
{
Name: "main2.gno",
Body: `
package main
import "gno.land/r/demo/multitxtest"
import "std"
func main2() {
multitxtest.Push()
}
`,
},
},
},
Entrypoint: "main2",
},
{
Pkg: &std.MemPackage{
Name: pkgName,
Path: pkgPath,
Files: []*std.MemFile{
{
Name: "main3.gno",
Body: `
package main
import "gno.land/r/demo/multitxtest"
import "std"
func main3() {
slice := multitxtest.GetSlice()
if len(slice) != 1 || slice[0] != "new-element" {
panic("pop/push is borked")
}
}
`,
},
},
},
Entrypoint: "main3",
},
})
}
15 changes: 15 additions & 0 deletions examples/gno.land/r/demo/multitxtest/tests_multi.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package multitxtest

var slice = []string{"undead-element"}

func Pop() {
slice = slice[:len(slice)-1]
}

func Push() {
slice = append(slice, "new-element")
}

func GetSlice() []string {
return slice
}
61 changes: 31 additions & 30 deletions gnovm/pkg/gnolang/realm.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,57 +121,58 @@ func (rlm *Realm) String() string {
//----------------------------------------
// ownership hooks

// po's old elem value is xo, will become co.
// po, xo, and co may each be nil.
// if rlm or po is nil, do nothing.
// xo or co is nil if the element value is undefined or has no
// base's old elem value is oldValue, will become newValue.
// base, oldValue, and newValue may each be nil.
// if rlm or base is nil, do nothing.
// oldValue or newValue is nil if the element value is undefined or has no
// associated object.
func (rlm *Realm) DidUpdate(po, xo, co Object) {
func (rlm *Realm) DidUpdate(base, oldValue, newValue Object) {
if rlm == nil {
return
}
if debug {
if co != nil && co.GetIsDeleted() {
if newValue != nil && newValue.GetIsDeleted() {
panic("cannot attach a deleted object")
}
if po != nil && po.GetIsTransient() {
if base != nil && base.GetIsTransient() {
panic("should not happen")
}
if po != nil && po.GetIsDeleted() {
if base != nil && base.GetIsDeleted() {
panic("cannot attach to a deleted object")
}
}
if po == nil || !po.GetIsReal() {
if base == nil || !base.GetIsReal() {
return // do nothing.
}
if po.GetObjectID().PkgID != rlm.ID {
if base.GetObjectID().PkgID != rlm.ID {
panic("cannot modify external-realm or non-realm object")
}
// From here on, po is real (not new-real).
// From here on, base is real (not new-real).
// Updates to .newCreated/.newEscaped /.newDeleted made here. (first gen)
// More appends happen during FinalizeRealmTransactions(). (second+ gen)
rlm.MarkDirty(po)
if co != nil {
co.IncRefCount()
if co.GetRefCount() > 1 {
if co.GetIsEscaped() {
// already escaped
} else {
rlm.MarkNewEscaped(co)
}
} else if co.GetIsReal() {
rlm.MarkDirty(co)
rlm.MarkDirty(base)

updateRefs := oldValue != newValue

if newValue != nil {
if updateRefs {
newValue.IncRefCount()
}
if newValue.GetRefCount() > 1 && !newValue.GetIsEscaped() {
rlm.MarkNewEscaped(newValue)
} else if newValue.GetIsReal() {
rlm.MarkDirty(newValue)
} else {
co.SetOwner(po)
rlm.MarkNewReal(co)
newValue.SetOwner(base)
rlm.MarkNewReal(newValue)
}
}
if xo != nil {
xo.DecRefCount()
if xo.GetRefCount() == 0 {
if xo.GetIsReal() {
rlm.MarkNewDeleted(xo)
}
if oldValue != nil {
if updateRefs {
oldValue.DecRefCount()
}
if oldValue.GetRefCount() == 0 && oldValue.GetIsReal() {
rlm.MarkNewDeleted(oldValue)
}
}
}
Expand Down
Loading