[camera] Improve the stability of WebRTC session establishment (#39118)
* [camera] Improve the stability of WebRTC session establishment
* Fix memory leak
diff --git a/examples/camera-controller/args.gni b/examples/camera-controller/args.gni
index 5f38802..fc40544 100644
--- a/examples/camera-controller/args.gni
+++ b/examples/camera-controller/args.gni
@@ -34,3 +34,4 @@
chip_tlv_validate_char_string_on_write = true
chip_build_controller_dynamic_server = true
+chip_device_config_enable_wifipaf = false
diff --git a/examples/camera-controller/commands/common/CHIPCommand.h b/examples/camera-controller/commands/common/CHIPCommand.h
index b5f3c70..c9e2aab 100644
--- a/examples/camera-controller/commands/common/CHIPCommand.h
+++ b/examples/camera-controller/commands/common/CHIPCommand.h
@@ -99,10 +99,6 @@
AddArgument("commissioner-vendor-id", 0, UINT16_MAX, &mCommissionerVendorId,
"The vendor id to use for camera-controller. If not provided, chip::VendorId::TestVendor1 (65521, 0xFFF1) will "
"be used.");
- AddArgument("start-websocket-server", 0, 1, &mStartWebSocketServer,
- "Start the built‑in WebSocket server that exposes the interactive‑command API. "
- "If not provided or 0 (\"false\"), the WebSocket server is disabled. "
- "If 1 (\"true\"), the WebSocket server is started and listens on the default port.");
}
/////////// Command Interface /////////
@@ -164,7 +160,6 @@
PersistentStorage mCommissionerStorage;
#endif // CONFIG_USE_LOCAL_STORAGE
chip::Optional<char *> mLogFilePath;
- chip::Optional<bool> mStartWebSocketServer;
chip::PersistentStorageOperationalKeystore mOperationalKeystore;
chip::Credentials::PersistentStorageOpCertStore mOpCertStore;
static chip::Crypto::RawKeySessionKeystore sSessionKeystore;
diff --git a/examples/camera-controller/commands/interactive/InteractiveCommands.cpp b/examples/camera-controller/commands/interactive/InteractiveCommands.cpp
index 6cfa5d9..1bf29a6 100644
--- a/examples/camera-controller/commands/interactive/InteractiveCommands.cpp
+++ b/examples/camera-controller/commands/interactive/InteractiveCommands.cpp
@@ -44,22 +44,47 @@
// File pointer for the log file
FILE * sLogFile = nullptr;
+// Global flag to signal shutdown
+std::atomic<bool> sShutdownRequested(false);
+
std::queue<std::string> sCommandQueue;
std::mutex sQueueMutex;
std::condition_variable sQueueCondition;
void ReadCommandThread()
{
- char * command;
- while (true)
+ for (;;)
{
- command = readline(kInteractiveModePrompt);
- if (command != nullptr && *command)
+ // `readline()` allocates with `malloc()`. Wrap it in a smart
+ // pointer so we cannot leak it on every early–return/throw path.
+ std::unique_ptr<char, decltype(&std::free)> rawLine{ readline(kInteractiveModePrompt), &std::free };
+
+ if (!rawLine) // EOF or fatal error → shut down cleanly
{
- std::unique_lock<std::mutex> lock(sQueueMutex);
- sCommandQueue.push(command);
- free(command);
+ std::lock_guard<std::mutex> lk{ sQueueMutex };
+ sCommandQueue.emplace(kInteractiveModeStopCommand);
sQueueCondition.notify_one();
+ break;
+ }
+
+ // Ignore empty lines produced by just hitting <Enter>.
+ if (*rawLine == '\0')
+ continue;
+
+ // Copy into an owning `std::string` before the pointer vanishes.
+ std::string line{ rawLine.get() };
+
+ {
+ std::lock_guard<std::mutex> lk{ sQueueMutex };
+ sCommandQueue.push(line);
+ }
+ sQueueCondition.notify_one();
+
+ // Bail out when the user asks to quit.
+ if (line == kInteractiveModeStopCommand)
+ {
+ ChipLogProgress(NotSpecified, "ReadCommandThread exit on quit");
+ break;
}
}
}
@@ -114,7 +139,14 @@
std::string InteractiveStartCommand::GetCommand() const
{
std::unique_lock<std::mutex> lock(sQueueMutex);
- sQueueCondition.wait(lock, [&] { return !sCommandQueue.empty(); });
+
+ // Wait until queue is not empty OR shutdown is requested
+ sQueueCondition.wait(lock, [&] { return !sCommandQueue.empty() || sShutdownRequested.load(); });
+
+ if (sShutdownRequested.load())
+ {
+ return {}; // empty string signals caller to exit
+ }
std::string command = sCommandQueue.front();
sCommandQueue.pop();
@@ -161,25 +193,6 @@
Logging::SetLogRedirectCallback(LoggingCallback);
}
- bool startWebSocketServer = mStartWebSocketServer.ValueOr(false);
- if (startWebSocketServer)
- {
- InteractiveServerCommand * command =
- static_cast<InteractiveServerCommand *>(CommandMgr().GetCommandByName("interactive", "server"));
- if (command == nullptr)
- {
- ChipLogError(NotSpecified, "Interactive server command not found.");
- return CHIP_ERROR_INTERNAL;
- }
-
- CHIP_ERROR err = command->RunCommand();
- if (err != CHIP_NO_ERROR)
- {
- ChipLogError(NotSpecified, "Failed to run interactive server command: %" CHIP_ERROR_FORMAT, err.Format());
- return err;
- }
- }
-
std::thread readCommands(ReadCommandThread);
readCommands.detach();
@@ -187,7 +200,12 @@
while (true)
{
std::string command = GetCommand();
- if (!command.empty() && !ParseCommand(command, &status))
+ if (command.empty())
+ {
+ break;
+ }
+
+ if (!ParseCommand(command, &status))
{
break;
}
@@ -438,6 +456,8 @@
gInteractiveServerResult.Reset();
SetCommandExitStatus(CHIP_NO_ERROR);
+ CloseLogFile();
+
return CHIP_NO_ERROR;
}
@@ -466,6 +486,11 @@
return shouldStop;
}
+void InteractiveServerCommand::StopCommand()
+{
+ mWebSocketServer.Stop();
+}
+
CHIP_ERROR InteractiveServerCommand::LogJSON(const char * json)
{
gInteractiveServerResult.MaybeAddResult(json);
diff --git a/examples/camera-controller/commands/interactive/InteractiveCommands.h b/examples/camera-controller/commands/interactive/InteractiveCommands.h
index 418324c..cacf2ef 100644
--- a/examples/camera-controller/commands/interactive/InteractiveCommands.h
+++ b/examples/camera-controller/commands/interactive/InteractiveCommands.h
@@ -87,6 +87,8 @@
/////////// RemoteDataModelLoggerDelegate interface /////////
CHIP_ERROR LogJSON(const char * json) override;
+ void StopCommand();
+
private:
WebSocketServer mWebSocketServer;
chip::Optional<uint16_t> mPort;
diff --git a/examples/camera-controller/main.cpp b/examples/camera-controller/main.cpp
index 0186d11..cf622da 100644
--- a/examples/camera-controller/main.cpp
+++ b/examples/camera-controller/main.cpp
@@ -39,8 +39,8 @@
// Convert command line arguments to a vector of strings for easier manipulation
std::vector<std::string> args(argv, argv + argc);
- // Check if "interactive" and "start" are not in the arguments
- if (args.size() < 3 || args[1] != "interactive" || args[2] != "start")
+ // Check if "interactive" is not in the arguments
+ if (args.size() < 3 || args[1] != "interactive")
{
// Insert "interactive" and "start" after the executable name
args.insert(args.begin() + 1, "interactive");
diff --git a/examples/common/websocket-server/WebSocketServer.cpp b/examples/common/websocket-server/WebSocketServer.cpp
index 402754d..60c366a 100644
--- a/examples/common/websocket-server/WebSocketServer.cpp
+++ b/examples/common/websocket-server/WebSocketServer.cpp
@@ -182,26 +182,44 @@
};
info.retry_and_idle_policy = &retry;
- auto context = lws_create_context(&info);
- VerifyOrReturnError(nullptr != context, CHIP_ERROR_INTERNAL);
+ mContext = lws_create_context(&info);
+ VerifyOrReturnError(mContext != nullptr, CHIP_ERROR_INTERNAL);
mRunning = true;
mDelegate = delegate;
while (mRunning)
{
- lws_service(context, -1);
+ lws_service(mContext, -1);
std::lock_guard<std::mutex> lock(gMutex);
- if (gMessageQueue.size())
+ if (!gMessageQueue.empty())
{
lws_callback_on_writable(gWebSocketInstance);
}
}
- lws_context_destroy(context);
+
+ lws_context_destroy(mContext);
+ mContext = nullptr;
return CHIP_NO_ERROR;
}
+void WebSocketServer::Stop()
+{
+ if (!mRunning)
+ {
+ return;
+ }
+
+ mRunning = false;
+
+ // Wake the poll/sleep inside lws_service()
+ if (mContext != nullptr)
+ {
+ lws_cancel_service(mContext);
+ }
+}
+
bool WebSocketServer::OnWebSocketMessageReceived(char * msg)
{
auto shouldContinue = mDelegate->OnWebSocketMessageReceived(msg);
diff --git a/examples/common/websocket-server/WebSocketServer.h b/examples/common/websocket-server/WebSocketServer.h
index b6a181c..7f7c239 100644
--- a/examples/common/websocket-server/WebSocketServer.h
+++ b/examples/common/websocket-server/WebSocketServer.h
@@ -30,10 +30,12 @@
public:
CHIP_ERROR Run(chip::Optional<uint16_t> port, WebSocketServerDelegate * delegate);
void Send(const char * msg);
+ void Stop();
bool OnWebSocketMessageReceived(char * msg) override;
private:
bool mRunning;
+ struct lws_context * mContext = nullptr;
WebSocketServerDelegate * mDelegate = nullptr;
};
