Refactor input for safe concurrency (#278)

* Extract input stuff

* Move d-pad option into the main screen

* Add mutex lock on the whole input user sessions struct

pkg/emulator/libretro/nanoarch/input.go

@@ -0,0 +1,97 @@
+package nanoarch
+
+import "sync"
+
+const (
+	// how many axes on the D-pad
+	dpadAxesNum = 4
+	// the upper limit on how many controllers (players)
+	// are possible for one play session (emulator instance)
+	controllersNum = 8
+)
+
+const (
+	InputTerminate = 0xFFFF
+)
+
+type Players struct {
+	session playerSession
+}
+
+type playerSession struct {
+	sync.RWMutex
+
+	state map[string][]controllerState
+}
+
+type controllerState struct {
+	keyState uint16
+	axes     [dpadAxesNum]int16
+}
+
+func NewPlayerSessionInput() Players {
+	return Players{
+		session: playerSession{
+			state: map[string][]controllerState{},
+		},
+	}
+}
+
+// close terminates user input session.
+func (ps *playerSession) close(id string) {
+	ps.Lock()
+	defer ps.Unlock()
+
+	delete(ps.state, id)
+}
+
+// setInput sets input state for some player in a game session.
+func (ps *playerSession) setInput(id string, player int, buttons uint16, dpad []byte) {
+	ps.Lock()
+	defer ps.Unlock()
+
+	if _, ok := ps.state[id]; !ok {
+		ps.state[id] = make([]controllerState, controllersNum)
+	}
+
+	ps.state[id][player].keyState = buttons
+	for i, axes := 0, len(dpad); i < dpadAxesNum && (i+1)*2+1 < axes; i++ {
+		axis := (i + 1) * 2
+		ps.state[id][player].axes[i] = int16(dpad[axis+1])<<8 + int16(dpad[axis])
+	}
+}
+
+// isKeyPressed checks if some button is pressed by any player.
+func (p *Players) isKeyPressed(player uint, key int) (pressed bool) {
+	p.session.RLock()
+	defer p.session.RUnlock()
+
+	for k := range p.session.state {
+		if ((p.session.state[k][player].keyState >> uint(key)) & 1) == 1 {
+			return true
+		}
+	}
+	return
+}
+
+// isDpadTouched checks if D-pad is used by any player.
+func (p *Players) isDpadTouched(player uint, axis uint) (shift int16) {
+	p.session.RLock()
+	defer p.session.RUnlock()
+
+	for k := range p.session.state {
+		value := p.session.state[k][player].axes[axis]
+		if value != 0 {
+			return value
+		}
+	}
+	return
+}
+
+type InputEvent struct {
+	RawState  []byte
+	PlayerIdx int
+	ConnID    string
+}
+
+func (ie InputEvent) bitmap() uint16 { return uint16(ie.RawState[1])<<8 + uint16(ie.RawState[0]) }

pkg/emulator/libretro/nanoarch/input_test.go

@@ -0,0 +1,27 @@
+package nanoarch
+
+import (
+	"math/rand"
+	"testing"
+)
+
+func TestConcurrentInput(t *testing.T) {
+	players := NewPlayerSessionInput()
+
+	session := "mad-test-session"
+	events := 1000
+	go func() {
+		for i := 0; i < events*2; i++ {
+			player := rand.Intn(controllersNum)
+			go players.session.setInput(session, player, 100, []byte{})
+			// here it usually crashes
+			go players.session.close(session)
+		}
+	}()
+	go func() {
+		for i := 0; i < events*2; i++ {
+			player := rand.Intn(controllersNum)
+			go players.isKeyPressed(uint(player), 100)
+		}
+	}()
+}

pkg/emulator/libretro/nanoarch/naemulator.go

@@ -50,13 +50,6 @@ void coreLog_cgo(enum retro_log_level level, const char *msg);
 */
 import "C"
 
-const numAxes = 4
-
-type controllerState struct {
-	keyState uint16
-	axes     [numAxes]int16
-}
-
 // naEmulator implements CloudEmulator
 type naEmulator struct {
 	sync.Mutex
@@ -73,8 +66,9 @@ type naEmulator struct {
 	isSavingLoading bool
 	storage         Storage
 
-	controllersMap map[string][]controllerState
-	done           chan struct{}
+	players Players
+
+	done chan struct{}
 }
 
 type Storage struct {
@@ -93,12 +87,6 @@ type VideoExporter struct {
 	imageChannel chan<- GameFrame
 }
 
-type InputEvent struct {
-	RawState  []byte
-	PlayerIdx int
-	ConnID    string
-}
-
 // GameFrame contains image and timeframe
 type GameFrame struct {
 	Image     *image.RGBA
@@ -108,10 +96,6 @@ type GameFrame struct {
 var NAEmulator *naEmulator
 var outputImg *image.RGBA
 
-const maxPort = 8
-
-const SocketAddrTmpl = "/tmp/cloudretro-retro-%s.sock"
-
 // NAEmulator implements CloudEmulator interface based on NanoArch(golang RetroArch)
 func NewNAEmulator(roomID string, inputChannel <-chan InputEvent, storage Storage, conf config.LibretroCoreConfig) (*naEmulator, chan GameFrame, chan []int16) {
 	imageChannel := make(chan GameFrame, 30)
@@ -127,19 +111,19 @@ func NewNAEmulator(roomID string, inputChannel <-chan InputEvent, storage Storag
 			HasMultitap:   conf.HasMultitap,
 			AutoGlContext: conf.AutoGlContext,
 		},
-		storage:        storage,
-		imageChannel:   imageChannel,
-		audioChannel:   audioChannel,
-		inputChannel:   inputChannel,
-		controllersMap: map[string][]controllerState{},
-		roomID:         roomID,
-		done:           make(chan struct{}, 1),
+		storage:      storage,
+		imageChannel: imageChannel,
+		audioChannel: audioChannel,
+		inputChannel: inputChannel,
+		players:      NewPlayerSessionInput(),
+		roomID:       roomID,
+		done:         make(chan struct{}, 1),
 	}, imageChannel, audioChannel
 }
 
 // NewVideoExporter creates new video Exporter that produces to unix socket
 func NewVideoExporter(roomID string, imgChannel chan GameFrame) *VideoExporter {
-	sockAddr := fmt.Sprintf(SocketAddrTmpl, roomID)
+	sockAddr := fmt.Sprintf("/tmp/cloudretro-retro-%s.sock", roomID)
 
 	go func(sockAddr string) {
 		log.Println("Dialing to ", sockAddr)
@@ -161,10 +145,7 @@ func NewVideoExporter(roomID string, imgChannel chan GameFrame) *VideoExporter {
 		}
 	}(sockAddr)
 
-	return &VideoExporter{
-		imageChannel: imgChannel,
-	}
-
+	return &VideoExporter{imageChannel: imgChannel}
 }
 
 // Init initialize new RetroArch cloud emulator
@@ -182,26 +163,17 @@ func Init(roomID string, withImageChannel bool, inputChannel <-chan InputEvent,
 	return emu, imageChannel, audioChannel
 }
 
+// listenInput handles user input.
+// The user input is encoded as bitmap that we decode
+// and send into the game emulator.
 func (na *naEmulator) listenInput() {
-	// input from javascript follows bitmap. Ex: 00110101
-	// we decode the bitmap and send to channel
-	for inpEvent := range NAEmulator.inputChannel {
-		inpBitmap := uint16(inpEvent.RawState[1])<<8 + uint16(inpEvent.RawState[0])
-
-		if inpBitmap == 0xFFFF {
-			// terminated
-			delete(na.controllersMap, inpEvent.ConnID)
+	for in := range NAEmulator.inputChannel {
+		bitmap := in.bitmap()
+		if bitmap == InputTerminate {
+			na.players.session.close(in.ConnID)
 			continue
 		}
-
-		if _, ok := na.controllersMap[inpEvent.ConnID]; !ok {
-			na.controllersMap[inpEvent.ConnID] = make([]controllerState, maxPort)
-		}
-
-		na.controllersMap[inpEvent.ConnID][inpEvent.PlayerIdx].keyState = inpBitmap
-		for i := 0; i < numAxes && (i+1)*2+1 < len(inpEvent.RawState); i++ {
-			na.controllersMap[inpEvent.ConnID][inpEvent.PlayerIdx].axes[i] = int16(inpEvent.RawState[(i+1)*2+1])<<8 + int16(inpEvent.RawState[(i+1)*2])
-		}
+		na.players.session.setInput(in.ConnID, in.PlayerIdx, bitmap, in.RawState)
 	}
 }
 
@@ -218,7 +190,7 @@ func (na *naEmulator) SetViewport(width int, height int) {
 }
 
 func (na *naEmulator) Start() {
-	na.playGame(na.gamePath)
+	na.playGame()
 	ticker := time.NewTicker(time.Second / time.Duration(na.meta.Fps))
 
 	for range ticker.C {
@@ -239,7 +211,7 @@ func (na *naEmulator) Start() {
 	}
 }
 
-func (na *naEmulator) playGame(path string) {
+func (na *naEmulator) playGame() {
 	// When start game, we also try loading if there was a saved state
 	na.LoadGame()
 }

pkg/emulator/libretro/nanoarch/nanoarch.go

@@ -182,11 +182,9 @@ func coreInputState(port C.unsigned, device C.unsigned, index C.unsigned, id C.u
 			return 0
 		}
 		axis := index*2 + id
-		for k := range NAEmulator.controllersMap {
-			value := NAEmulator.controllersMap[k][port].axes[axis]
-			if value != 0 {
-				return (C.int16_t)(value)
-			}
+		value := NAEmulator.players.isDpadTouched(uint(port), uint(axis))
+		if value != 0 {
+			return (C.int16_t)(value)
 		}
 	}
 
@@ -200,12 +198,10 @@ func coreInputState(port C.unsigned, device C.unsigned, index C.unsigned, id C.u
 		return 0
 	}
 
-	// check if any player is pressing that key
-	for k := range NAEmulator.controllersMap {
-		if ((NAEmulator.controllersMap[k][port].keyState >> uint(key)) & 1) == 1 {
-			return 1
-		}
+	if NAEmulator.players.isKeyPressed(uint(port), key) {
+		return 1
 	}
+
 	return 0
 }
 

pkg/emulator/libretro/nanoarch/nanoarch_test.go

@@ -91,9 +91,9 @@ func GetEmulatorMock(room string, system string) *EmulatorMock {
 				UsesLibCo:   meta.UsesLibCo,
 				HasMultitap: meta.HasMultitap,
 			},
-			controllersMap: map[string][]controllerState{},
-			roomID:         room,
-			done:           make(chan struct{}, 1),
+			players: NewPlayerSessionInput(),
+			roomID:  room,
+			done:    make(chan struct{}, 1),
 		},
 
 		canvas: image.NewRGBA(image.Rect(0, 0, meta.Width, meta.Height)),

web/css/main.css

@@ -368,7 +368,7 @@ body {
 
 #room-txt {
     position: absolute;
-    width: 98px;
+    width: 59px;
     top: 45px;
     left: 23px;
     color: #bababa;
@@ -679,7 +679,7 @@ body {
   width: 35px;
   height: 20px;
 
-  top: 15px;
+  top: 44px;
   left: 85px;
 }
 

web/css/ui.css

@@ -4,9 +4,8 @@
 }
 
 #btn-settings {
-    position: relative;
     top: 14px;
-    left: 62px;
+    left: 70px;
 }
 
 .modal-window {

web/index.html

@@ -72,6 +72,11 @@
     <!-- TODO: remove -->
     <input id="room-txt" type="text" placeholder="room id..." unselectable="on" class=" unselectable" disabled>
 
+    <label class="dpad-toggle-label" title="D-pad toggle">
+        <input type="checkbox" id="dpad-toggle" checked>
+        <span class="dpad-toggle-slider"></span>
+    </label>
+
     <div id="noti-box" unselectable="on" class="unselectable">Oh my god</div>
 
     <div id="help-overlay">
@@ -99,10 +104,6 @@
             * -- applied after application restart
         </div>
     </div>
-    <label class="dpad-toggle-label">
-        <input type="checkbox" id="dpad-toggle" checked>
-        <span class="dpad-toggle-slider"></span>
-    </label>
 </div>
 
 <a id="ribbon" style="position: fixed; right: 0; top: 0;" href="https://github.com/giongto35/cloud-game"><img