source-academy / go-slang

Implementation of Go
Apache License 2.0
0 stars 1 forks source link

Concurrency bug: argument passed into goroutine is not evaluated first before being passed by value #32

Open chengda300 opened 1 month ago

chengda300 commented 1 month ago

When the time quantum is sufficiently large (say 30 instructions), the variable being passed as an argument in the creation of a new goroutine may be modified by the calling goroutine as it is not evaluated at the time when the new goroutine is created.

Since context is not switched yet as the time quantum is not reached, the calling goroutine can continue running before the newly created goroutine can run. The calling goroutine modifies the value of the variable(s) being passed into the goroutine's argument(s), resulting in the new goroutine using the arguments of possibly wrong values, despite it being supposed to be "passed by value", which means that the argument is not evaluated yet.

Example:

package main
import "fmt"
func add(a int){
    fmt.Println(a);
}

func main() {
   for i := 0; i < 10; i++ {
      go add(i);
   }
   fmt.Println("Done");
}

should print 0\n1\n2\n3\n4\n5\n6\n7\n8\n9\nDone\n However, it prints 2\n2\n2\n5\n5\n7\n7\n9\n9\n9\nDone\n as the loop incremented i before the goroutine can run, so the goroutine created when i = 0,1 was only run when i = 2 in the loop, so it prints 2 instead of 0 or 1.

chengda300 commented 3 weeks ago

Go statements should create a "Go (argCount)" instead of "Call (argCount)" instruction, so the current implementation is actually wrong (Basically it should follow the same logic as a function call but execute the function in a new thread, then the environment would be its own environment, just like in a function call)