Commit Graph
4 Commits
Author SHA1 Message Date
mpl a2f56431ea interp: fix data race for composite literal creation
This change fixes two data races related to composite literal creation.

The first one isn't controversial as it is just about initializing the
variable that contains the values in the right place, i.e. within the
n.exec, so that this variable is local to each potential goroutine,
instead of being racily shared by all goroutines.

The second one is more worrying, i.e. having to protect the node typ
with a mutex, because a call to func (t *itype) refType actually
modifies the itype itself, which means it is not concurrent safe.
The change seems to work, and does not seem to introduce regression, but
it is still a concern as it probably is a sign that more similar
guarding has to be done in several other places.
2020-09-14 16:22:04 +02:00
mpl 04770a4b81 interp: fix data races (#839)
This change fixes two distinct data races:

1) some global vars of type *itype of the interp package are actually
mutated during the lifecycle of an Interpreter. Even worse: if more than
one Interpreter instance are created and used at a given time, they are
actually racing each other for these global vars.
Therefore, this change replaces these global vars with generator
functions that create the needed type on the fly.

2) the symbols given as argument of Interpreter.Use were directly copied
as reference (since they're maps) when mapped inside an Interpreter
instance. Since the usual case is to give the symbols from the stdlib
package, it means when the interpreter mutates its own symbols in
fixStdio, it would actually mutate the corresponding global vars of the
stdlib package. Again, this is at least racy as soon as several
instances of an Intepreter are concurrently running.
This change fixes the race by making sure Interpreter.Use actually
copies the symbol values instead of copying the references.
2020-09-09 11:59:07 +02:00
mplandMarc Vertes 896bfeb5a1 interp: new EvalPath API
The recent changes that added some redeclaration checks implicitly added more
strictness related to namespaces and scopes which, among other things, broke
some uses that "accidentally" used to work.

For example, given

const script1 = `
	import "fmt"

	// more code
`
const script2 = `
	import "fmt"

	// some other code
`
If one Evals script1, then script2, with the same interpreter, without
specifying any scope, as the two fragments would be considered part of the same
(.go file) scope by default, a redeclaration error would be triggered because
import "fmt" is seen twice.

A work-around would have been to specify (a different) i.Name before each Eval
call, so that each script is considered as coming from a different .go file, and
hence are respectively in different scopes with respect to imports.

That lead us to realize we had to make specifying things such as file-scope, and
"incremental mode" (aka REPL), more obvious in the context of an Eval call.

In addition, we want to lay down the foundations for Yaegi being able to behave
more like the go tool wrt to various inputs, i.e. it should be able to take a
package directory, or an import path, as input, instead of just a .go file.

Hence the introduction of a new kind of Eval method (whose signature is not fixed yet):

func (interp *Interpreter) EvalPath(path string) (res reflect.Value, err error)

It partially solves the problem described above because:

1. the path given to EvalPath can be used as the file-scope hint mentioned
above, for now (even though the related implementation details might change).
2. Eval always runs in incremental mode, whereas EvalPath always runs in
non-incremental mode, hence clarifying the situation in that respect.

And to avoid confusion, the Name field of Interpreter is now non-exported,
since it is somewhat redundant with the path argument of EvalPath.

Note that #731 is not fully fixed (and might never be), as a requirement of the
proposed solution is to move the input bits of code into respective files
(instead of leaving them as strings).

Finally, some related bugfixes, documention changes, and some refactoring have
been included. Notably, there is no "empty scope" anymore, i.e. name defaults
to "_.go" when it is not specified.

Updates #731
Fixes #778
Fixes #798
Fixes #789 

Co-authored-by: Marc Vertes <mvertes@free.fr>
2020-08-20 13:14:15 +02:00
mpl 563270ca02 interp: support yet another vendoring case
* interp: support another vendoring case

Namely, when the vendor dir is a sibling (or an uncle) relative to the
current pkg

Fixes #758

* make linter happier

* address review comments

* fix, cleanup, add unit tests

* add dummy files to force dirs into git
2020-07-15 15:35:04 +02:00