Fix data race in childProcess (#82)

There was a data race between GetResourceConsumption and watchResourceConsumption,
one was setting resourceSpec, the other was reading.

Now setting of resourceSpec is done before the resource motoring is started and
both functions only read it so there is no more data race.

Issue: #81

Author: tigrannajaryan

testbed/testbed/child_process.go

@@ -48,6 +48,12 @@ type resourceSpec struct {
 	resourceCheckPeriod time.Duration
 }
 
+// isSpecified returns true if any part of resourceSpec is specified,
+// i.e. has non-zero value.
+func (rs *resourceSpec) isSpecified() bool {
+	return rs != nil && (rs.expectedMaxCPU != 0 || rs.expectedMaxRAM != 0)
+}
+
 // childProcess is a child process that can be monitored and the output
 // of which will be written to a log file.
 type childProcess struct {
@@ -101,10 +107,11 @@ type childProcess struct {
 }
 
 type startParams struct {
-	name        string
-	logFilePath string
-	cmd         string
-	cmdArgs     []string
+	name         string
+	logFilePath  string
+	cmd          string
+	cmdArgs      []string
+	resourceSpec *resourceSpec
 }
 
 type ResourceConsumption struct {
@@ -126,6 +133,7 @@ func (cp *childProcess) start(params startParams) error {
 
 	cp.name = params.name
 	cp.doneSignal = make(chan struct{})
+	cp.resourceSpec = params.resourceSpec
 
 	log.Printf("Starting %s (%s)", cp.name, params.cmd)
 
@@ -236,8 +244,11 @@ func (cp *childProcess) stop() {
 	})
 }
 
-func (cp *childProcess) watchResourceConsumption(spec *resourceSpec) error {
-	cp.resourceSpec = spec
+func (cp *childProcess) watchResourceConsumption() error {
+	if !cp.resourceSpec.isSpecified() {
+		// Resource monitoring is not enabled.
+		return nil
+	}
 
 	var err error
 	cp.processMon, err = process.NewProcess(int32(cp.cmd.Process.Pid))
@@ -257,7 +268,7 @@ func (cp *childProcess) watchResourceConsumption(spec *resourceSpec) error {
 	}
 
 	// Measure every resourceCheckPeriod.
-	ticker := time.NewTicker(spec.resourceCheckPeriod)
+	ticker := time.NewTicker(cp.resourceSpec.resourceCheckPeriod)
 	defer ticker.Stop()
 
 	for {
@@ -351,7 +362,7 @@ func (cp *childProcess) isAllowedResourceUsage() bool {
 
 // GetResourceConsumption returns resource consumption as a string
 func (cp *childProcess) GetResourceConsumption() string {
-	if cp.resourceSpec == nil {
+	if !cp.resourceSpec.isSpecified() {
 		// Monitoring is not enabled.
 		return ""
 	}

testbed/testbed/test_case.go

@@ -127,26 +127,25 @@ func (tc *TestCase) StartAgent() {
 	logFileName := tc.composeTestResultFileName("agent.log")
 
 	err := tc.agentProc.start(startParams{
-		name:        "Agent",
-		logFilePath: logFileName,
-		cmd:         testBedConfig.Agent,
-		cmdArgs:     []string{"--config", tc.agentConfigFile},
+		name:         "Agent",
+		logFilePath:  logFileName,
+		cmd:          testBedConfig.Agent,
+		cmdArgs:      []string{"--config", tc.agentConfigFile},
+		resourceSpec: &tc.resourceSpec,
 	})
 
 	if err != nil {
 		tc.indicateError(err)
 		return
 	}
 
-	// Start watching if expected resources are defined.
-	if tc.resourceSpec.expectedMaxCPU != 0 || tc.resourceSpec.expectedMaxRAM != 0 {
-		go func() {
-			err := tc.agentProc.watchResourceConsumption(&tc.resourceSpec)
-			if err != nil {
-				tc.indicateError(err)
-			}
-		}()
-	}
+	// Start watching resource consumption.
+	go func() {
+		err := tc.agentProc.watchResourceConsumption()
+		if err != nil {
+			tc.indicateError(err)
+		}
+	}()
 
 	// Wait a bit for agent to start. This is a hack. We need to have a way to
 	// wait for agent to start properly.