Refactor thread handling to fix OS X bug.

On OS X, openL2CAPChannelSync registers events on the current
thread's run loop, so Connect needs to be called on a thread that's
going to do CFRunLoopRun; this was causing all Wiimote input to be
ignored.  Easiest way to do that is to use the Wiimote thread, and
have Read call CFRunLoopRun to block on events, bringing OS X's
Wiimote event loop in line with every other platform's.  This also
means that the thread can't be stopped and recreated by Prepare,
so make Prepare notify it instead, which has the side effect of not
making the GUI block on Prepare.  (It would be nice if the GUI also
did not block on searching for devices, because blocking the GUI
is gross, but for now...)

Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp

@@ -43,12 +43,12 @@ bool WiimoteScanner::IsReady() const
 	return false;
 }
 
-bool Wiimote::Connect()
+bool Wiimote::ConnectInternal()
 {
 	return 0;
 }
 
-void Wiimote::Disconnect()
+void Wiimote::DisconnectInternal()
 {
 	return;
 }

Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp

@@ -27,8 +27,7 @@ namespace WiimoteReal
 {
 
 WiimoteScanner::WiimoteScanner()
-	: m_run_thread()
+	: m_want_wiimotes()
-	, m_want_wiimotes()
 	, device_id(-1)
 	, device_sock(-1)
 {
@@ -135,7 +134,7 @@ void WiimoteScanner::FindWiimotes(std::vector<Wiimote*> & found_wiimotes, Wiimot
 }
 
 // Connect to a wiimote with a known address.
-bool Wiimote::Connect()
+bool Wiimote::ConnectInternal()
 {
 	sockaddr_l2 addr;
 	addr.l2_family = AF_BLUETOOTH;
@@ -168,7 +167,7 @@ bool Wiimote::Connect()
 	return true;
 }
 
-void Wiimote::Disconnect()
+void Wiimote::DisconnectInternal()
 {
 	close(cmd_sock);
 	close(int_sock);

Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp

@@ -459,7 +459,7 @@ bool WiimoteScanner::IsReady() const
 }
 
 // Connect to a wiimote with a known device path.
-bool Wiimote::Connect()
+bool Wiimote::ConnectInternal()
 {
 	if (IsConnected())
 		return false;
@@ -535,7 +535,7 @@ bool Wiimote::Connect()
 	return true;
 }
 
-void Wiimote::Disconnect()
+void Wiimote::DisconnectInternal()
 {
 	if (!IsConnected())
 		return;

Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm

@@ -64,7 +64,7 @@
 		return;
 	}
 
-	if (wm->inputlen != 0) {
+	if (wm->inputlen != -1) {
 		WARN_LOG(WIIMOTE, "Dropping packet for wiimote %i, queue full",
 				wm->index + 1);
 		return;
@@ -73,9 +73,8 @@
 	memcpy(wm->input, data, length);
 	wm->inputlen = length;
 
-	(void)wm->Read();
-
 	(void)UpdateSystemActivity(UsrActivity);
+	CFRunLoopStop(CFRunLoopGetCurrent());
 }
 
 - (void) l2capChannelClosed: (IOBluetoothL2CAPChannel *) l2capChannel
@@ -100,7 +99,7 @@
 
 	WARN_LOG(WIIMOTE, "Lost channel to wiimote %i", wm->index + 1);
 
-	wm->Disconnect();
+	wm->DisconnectInternal();
 }
 @end
 
@@ -189,7 +188,7 @@ bool WiimoteScanner::IsReady() const
 }
 
 // Connect to a wiimote with a known address.
-bool Wiimote::Connect()
+bool Wiimote::ConnectInternal()
 {
 	if (IsConnected())
 		return false;
@@ -204,7 +203,7 @@ bool Wiimote::Connect()
 	{
 		ERROR_LOG(WIIMOTE, "Unable to open L2CAP channels "
 			"for wiimote %i", index + 1);
-		Disconnect();
+		DisconnectInternal();
 		
 		[cbt release];
 		return false;
@@ -220,7 +219,7 @@ bool Wiimote::Connect()
 }
 
 // Disconnect a wiimote.
-void Wiimote::Disconnect()
+void Wiimote::DisconnectInternal()
 {
 	if (ichan != NULL)
 		[ichan release];
@@ -250,16 +249,12 @@ bool Wiimote::IsConnected() const
 
 int Wiimote::IORead(unsigned char *buf)
 {
-	int bytes;
+	input = buf;
+	inputlen = -1;
 
-	if (!IsConnected())
+	CFRunLoopRun();
-		return 0;
 
-	bytes = inputlen;
+	return inputlen;
-	memcpy(buf, input, bytes);
-	inputlen = 0;
-
-	return bytes;
 }
 
 int Wiimote::IOWrite(const unsigned char *buf, int len)

Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp

@@ -40,7 +40,7 @@ WiimoteScanner g_wiimote_scanner;
 Wiimote::Wiimote()
 	: index()
 #ifdef __APPLE__
-	, btd(), ichan(), cchan(), inputlen(), m_connected()
+	, btd(), ichan(), cchan(), input(), inputlen(), m_connected()
 #elif defined(__linux__) && HAVE_BLUEZ
 	, cmd_sock(-1), int_sock(-1)
 #elif defined(_WIN32)
@@ -49,7 +49,7 @@ Wiimote::Wiimote()
 	, m_last_input_report()
 	, m_channel(0)
 	, m_rumble_state()
-	, m_run_thread(false)
+	, m_need_prepare()
 {
 #if defined(__linux__) && HAVE_BLUEZ
 	bdaddr = (bdaddr_t){{0, 0, 0, 0, 0, 0}};
@@ -60,9 +60,6 @@ Wiimote::~Wiimote()
 {
 	StopThread();
 
-	if (IsConnected())
-		Disconnect();
-	
 	ClearReadQueue();
 	m_write_reports.Clear();
 }
@@ -225,8 +222,8 @@ bool Wiimote::Read()
 	}
 	else if (0 == result)
 	{
-		WARN_LOG(WIIMOTE, "Wiimote::IORead failed. Disconnecting Wiimote %d.", index + 1);
+		ERROR_LOG(WIIMOTE, "Wiimote::IORead failed. Disconnecting Wiimote %d.", index + 1);
-		Disconnect();
+		DisconnectInternal();
 	}
 
 	return false;
@@ -308,21 +305,25 @@ void Wiimote::Update()
 			rpt.data(), rpt.size());
 }
 
-bool Wiimote::Prepare(int _index)
+void Wiimote::Prepare(int _index)
 {
 	index = _index;
+	m_need_prepare = true;
+}
 
+bool Wiimote::PrepareOnThread()
+{
 	// core buttons, no continuous reporting
-	u8 const mode_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_REPORT_MODE, 0, WM_REPORT_CORE};
+	u8 static const mode_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_REPORT_MODE, 0, WM_REPORT_CORE};
 	
 	// Set the active LEDs and turn on rumble.
-	u8 const led_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_LEDS, u8(WIIMOTE_LED_1 << (index%WIIMOTE_BALANCE_BOARD) | 0x1)};
+	u8 static const led_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_LEDS, u8(WIIMOTE_LED_1 << (index%WIIMOTE_BALANCE_BOARD) | 0x1)};
 
 	// Turn off rumble
-	u8 rumble_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_RUMBLE, 0};
+	u8 static const rumble_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_RUMBLE, 0};
 	
 	// Request status report
-	u8 const req_status_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_REQUEST_STATUS, 0};
+	u8 static const req_status_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_REQUEST_STATUS, 0};
 	// TODO: check for sane response?
 
 	return (IOWrite(mode_report, sizeof(mode_report))
@@ -480,35 +481,75 @@ void WiimoteScanner::ThreadFunc()
 	NOTICE_LOG(WIIMOTE, "Wiimote scanning has stopped.");
 }
 
+bool Wiimote::Connect()
+{
+	m_thread_ready = false;
+	StartThread();
+	WaitReady();
+	return IsConnected();
+}
+
 void Wiimote::StartThread()
 {
-	m_run_thread = true;
 	m_wiimote_thread = std::thread(std::mem_fun(&Wiimote::ThreadFunc), this);
 }
 
 void Wiimote::StopThread()
 {
-	m_run_thread = false;
 	if (m_wiimote_thread.joinable())
 		m_wiimote_thread.join();
 }
 
+void Wiimote::SetReady()
+{
+	if (!m_thread_ready)
+	{
+		{
+			std::lock_guard<std::mutex> Guard(m_thread_ready_mutex);
+			m_thread_ready = true;
+		}
+		m_thread_ready_cond.notify_all();
+	}
+}
+
+void Wiimote::WaitReady()
+{
+	std::unique_lock<std::mutex> lock(m_thread_ready_mutex);
+	while (!m_thread_ready)
+	{
+		m_thread_ready_cond.wait(lock);
+	}
+}
+
 void Wiimote::ThreadFunc()
 {
 	Common::SetCurrentThreadName("Wiimote Device Thread");
+	bool ok = ConnectInternal();
+
+	SetReady();
+
+	if (!ok)
+	{
+		return;
+	}
 
 	// main loop
-	while (m_run_thread && IsConnected())
+	while (IsConnected())
 	{
-#ifdef __APPLE__
+		if (m_need_prepare)
-		// Reading happens elsewhere on OSX
+		{
-		bool const did_something = Write();
+			m_need_prepare = false;
-#else
+			if (!PrepareOnThread())
-		bool const did_something = Write() || Read();
+			{
-#endif
+				ERROR_LOG(WIIMOTE, "Wiimote::PrepareOnThread failed.  Disconnecting Wiimote %d.", index + 1);
-		if (!did_something)
+				DisconnectInternal();
-			Common::SleepCurrentThread(1);
 			}
+		}
+		Write();
+		Read();
+	}
+
+	DisconnectInternal();
 }
 
 void LoadSettings()
@@ -629,24 +670,32 @@ void ChangeWiimoteSource(unsigned int index, int source)
 		Host_ConnectWiimote(index, true);
 }
 
+static bool TryToConnectWiimoteN(Wiimote* wm, unsigned int i)
+{
+	if (WIIMOTE_SRC_REAL & g_wiimote_sources[i]
+		&& !g_wiimotes[i])
+	{
+		if (wm->Connect())
+		{
+			wm->Prepare(i);
+			NOTICE_LOG(WIIMOTE, "Connected to Wiimote %i.", i + 1);
+			g_wiimotes[i] = wm;
+			Host_ConnectWiimote(i, true);
+		}
+		return true;
+	}
+	return false;
+}
+
 void TryToConnectWiimote(Wiimote* wm)
 {
 	std::unique_lock<std::recursive_mutex> lk(g_refresh_lock);
 
 	for (unsigned int i = 0; i < MAX_WIIMOTES; ++i)
 	{
-		if (WIIMOTE_SRC_REAL & g_wiimote_sources[i]
+		if (TryToConnectWiimoteN(wm, i))
-			&& !g_wiimotes[i])
 		{
-			if (wm->Connect() && wm->Prepare(i))
+			wm = NULL;
-			{
-				NOTICE_LOG(WIIMOTE, "Connected to Wiimote %i.", i + 1);
-				
-				std::swap(g_wiimotes[i], wm);
-				g_wiimotes[i]->StartThread();
-				
-				Host_ConnectWiimote(i, true);
-			}
 			break;
 		}
 	}
@@ -662,18 +711,9 @@ void TryToConnectBalanceBoard(Wiimote* wm)
 {
 	std::unique_lock<std::recursive_mutex> lk(g_refresh_lock);
 
-	if (WIIMOTE_SRC_REAL & g_wiimote_sources[WIIMOTE_BALANCE_BOARD]
+	if (TryToConnectWiimoteN(wm, WIIMOTE_BALANCE_BOARD))
-		&& !g_wiimotes[WIIMOTE_BALANCE_BOARD])
 	{
-		if (wm->Connect() && wm->Prepare(WIIMOTE_BALANCE_BOARD))
+		wm = NULL;
-		{
-			NOTICE_LOG(WIIMOTE, "Connected to Balance Board %i.", WIIMOTE_BALANCE_BOARD + 1);
-			
-			std::swap(g_wiimotes[WIIMOTE_BALANCE_BOARD], wm);
-			g_wiimotes[WIIMOTE_BALANCE_BOARD]->StartThread();
-			
-			Host_ConnectWiimote(WIIMOTE_BALANCE_BOARD, true);
-		}
 	}
 	
 	g_wiimote_scanner.WantBB(0 != CalculateWantedBB());
@@ -687,26 +727,13 @@ void DoneWithWiimote(int index)
 {
 	std::lock_guard<std::recursive_mutex> lk(g_refresh_lock);
 
-	if (g_wiimotes[index])
+	Wiimote* wm = g_wiimotes[index];
-	{
-		g_wiimotes[index]->StopThread();
 
+	if (wm)
+	{
+		g_wiimotes[index] = NULL;
 		// First see if we can use this real Wiimote in another slot.
-		for (unsigned int i = 0; i < MAX_WIIMOTES; ++i)
+		TryToConnectWiimote(wm);
-		{
-			if (WIIMOTE_SRC_REAL & g_wiimote_sources[i]
-				&& !g_wiimotes[i])
-			{
-				if (g_wiimotes[index]->Prepare(i))
-				{
-					std::swap(g_wiimotes[i], g_wiimotes[index]);
-					g_wiimotes[i]->StartThread();
-					
-					Host_ConnectWiimote(i, true);
-				}
-				break;
-			}
-		}
 	}
 	
 	// else, just disconnect the Wiimote
@@ -763,9 +790,7 @@ void Refresh()
 		{
 			if (g_wiimotes[i])
 			{
-				g_wiimotes[i]->StopThread();
 				g_wiimotes[i]->Prepare(i);
-				g_wiimotes[i]->StartThread();
 			}
 		}
 

Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h

@@ -52,13 +52,17 @@ public:
 
 	// connecting and disconnecting from physical devices
 	// (using address inserted by FindWiimotes)
+	// these are called from the wiimote's thread.
+	bool ConnectInternal();
+	void DisconnectInternal();
+
 	bool Connect();
-	void Disconnect();
 
 	// TODO: change to something like IsRelevant
 	bool IsConnected() const;
 
-	bool Prepare(int index);
+	void Prepare(int index);
+	bool PrepareOnThread();
 
 	void DisableDataReporting();
 	void EnableDataReporting(u8 mode);
@@ -72,9 +76,10 @@ public:
 	IOBluetoothDevice *btd;
 	IOBluetoothL2CAPChannel *ichan;
 	IOBluetoothL2CAPChannel *cchan;
-	char input[MAX_PAYLOAD];
+	unsigned char* input;
 	int inputlen;
 	bool m_connected;
+	CFRunLoopRef m_wiimote_thread_run_loop;
 #elif defined(__linux__) && HAVE_BLUEZ
 	bdaddr_t bdaddr;					// Bluetooth address
 	int cmd_sock;						// Command socket
@@ -100,11 +105,16 @@ private:
 	int IOWrite(u8 const* buf, int len);
 
 	void ThreadFunc();
+	void SetReady();
+	void WaitReady();
 
 	bool m_rumble_state;
 	
-	bool				m_run_thread;
 	std::thread               m_wiimote_thread;
+	volatile bool             m_thread_ready;
+	volatile bool             m_need_prepare;
+	std::mutex                m_thread_ready_mutex;
+	std::condition_variable   m_thread_ready_cond;
 
 	Common::FifoQueue<Report>	m_read_reports;
 	Common::FifoQueue<Report>	m_write_reports;