Skip to content

Checking the coverity issue[Do Not Merge] #109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Sid2001-maker wants to merge 1 commit into from
Closed

Checking the coverity issue[Do Not Merge] #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/IJavaScriptContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
class IJavaScriptContext
{
public:
virtual ~IJavaScriptContext() = default;
virtual bool runScript(const char *script, bool isModule=true, std::string name="", const char *args = nullptr, bool isApplication=false) = 0;
virtual bool runFile(const char *file, const char* args, bool isApplication=false) = 0;
virtual std::string getUrl() = 0;
Expand Down
14 changes: 11 additions & 3 deletions include/JSRuntimeClient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,15 @@ class CommandInterface
if (derived.send(command))
{
std::unique_lock<std::mutex> lock(mResponseMutex);
mResponseCondition.wait_for(lock, std::chrono::seconds(5));
response = mLastResponse;
return true;
mResponseReceived = false;
bool gotResponse = mResponseCondition.wait_for(lock, std::chrono::seconds(5),
[this]() { return mResponseReceived; });

if (gotResponse)
{
response = mLastResponse;
return true;
}
}

return false;
Expand All @@ -63,6 +69,7 @@ class CommandInterface
{
std::lock_guard<std::mutex> lock(mResponseMutex);
mLastResponse = message;
mResponseReceived = true;
mResponseCondition.notify_one();
}

Expand All @@ -71,6 +78,7 @@ class CommandInterface
CommandInterface &operator=(const CommandInterface &) = delete;

std::string mLastResponse;
bool mResponseReceived = false;
std::mutex mResponseMutex;
std::condition_variable mResponseCondition;
};
Expand Down
62 changes: 36 additions & 26 deletions src/JSRuntimeClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ bool JSRuntimeClient::run()
t.detach();

std::unique_lock<std::mutex> lock(mStateMutex);
mStateCondition.wait_for(lock, std::chrono::seconds(5));
mStateCondition.wait_for(lock, std::chrono::seconds(5), [this]() {
return mState != "none";
});
return mState == "open";
}

Expand Down Expand Up @@ -152,37 +154,45 @@ void JSRuntimeClient::onClose(websocketpp::connection_hdl hdl)
#ifndef UNIT_TEST_BUILD
int main(int argc, char **argv)
{
std::string command;
std::string response;
try {
std::string command;
std::string response;

if (argc > 1)
{
NativeJSLogger::log(INFO, "Send input commands at ws://localhost:%s\n", std::to_string(WS_SERVER_PORT).c_str());
return -1;
}

JSRuntimeClient *client = JSRuntimeClient::getInstance();
client->initialize(WS_SERVER_PORT);
if (!client->run())
{
NativeJSLogger::log(ERROR, "Unable to connect to server\n");
return -1;
}
if (argc > 1)
{
NativeJSLogger::log(INFO, "Send input commands at ws://localhost:%s\n", std::to_string(WS_SERVER_PORT).c_str());
return -1;
}

while (client->getState() == "open" && std::getline(std::cin, command))
{
client->sendCommand(command, response);
if (!response.empty())
JSRuntimeClient *client = JSRuntimeClient::getInstance();
client->initialize(WS_SERVER_PORT);
if (!client->run())
{
NativeJSLogger::log(INFO, "Response: %s\n", response.c_str());
NativeJSLogger::log(ERROR, "Unable to connect to server\n");
return -1;
}
else

while (client->getState() == "open" && std::getline(std::cin, command))
{
NativeJSLogger::log(WARN, "Missing response\n");
break;
client->sendCommand(command, response);
if (!response.empty())
{
NativeJSLogger::log(INFO, "Response: %s\n", response.c_str());
}
else
{
NativeJSLogger::log(WARN, "Missing response\n");
break;
}
}
}

return 0;
return 0;
} catch (const std::exception& e) {
NativeJSLogger::log(ERROR, "Uncaught exception in main: %s\n", e.what());
return -1;
} catch (...) {
NativeJSLogger::log(ERROR, "Unknown exception caught in main\n");
return -1;
}
}
#endif
46 changes: 28 additions & 18 deletions src/JSRuntimeClientContainer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,36 @@
#include "NativeJSLogger.h"
int main()
{
std::string containerId = "com.sky.as.apps_TestApp";
const std::string basePath = "/opt/twocontext"; // constant base path
const std::vector<std::string> apps = {"app1", "app2"};

std::string ipAddress = JSRuntimeContainer::getContainerIpAddress(containerId);
if (ipAddress.empty()) {
NativeJSLogger::log(ERROR, "Failed to retrieve IP address for container");
return 1;
}
try {
std::string containerId = "com.sky.as.apps_TestApp";
const std::string basePath = "/opt/twocontext"; // constant base path
const std::vector<std::string> apps = {"app1", "app2"};

std::string ipAddress = JSRuntimeContainer::getContainerIpAddress(containerId);
if (ipAddress.empty()) {
NativeJSLogger::log(ERROR, "Failed to retrieve IP address for container\n");
return 1;
}

for (const auto &app : apps) {
std::string url = basePath + std::string("/") + app + std::string("/index.html");
if (access(url.c_str(), F_OK) == 0) {
std::string pathAppConfig = basePath + std::string("/") + app + std::string("/app.config");
std::string options = JSRuntimeContainer::parseAppConfig(pathAppConfig);
std::string message = JSRuntimeContainer::buildLaunchMessage(url, options);
JSRuntimeContainer::connectAndSend(ipAddress, message);
for (const auto &app : apps) {
std::string url = basePath + std::string("/") + app + std::string("/index.html");
if (access(url.c_str(), F_OK) == 0) {
std::string pathAppConfig = basePath + std::string("/") + app + std::string("/app.config");
std::string options = JSRuntimeContainer::parseAppConfig(pathAppConfig);
std::string message = JSRuntimeContainer::buildLaunchMessage(url, options);
JSRuntimeContainer::connectAndSend(ipAddress, message);
}
}
}

return 0;
return 0;
}
catch (const std::exception& e) {
NativeJSLogger::log(ERROR, "Exception in main: %s\n", e.what());
return 1;
}
catch (...) {
NativeJSLogger::log(ERROR, "Unknown exception in main\n");
return 1;
}
}

1 change: 1 addition & 0 deletions src/JSRuntimeServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class JsonWrap
if (!itm || !cJSON_IsNumber(itm))
{
std::cerr << "Error: " << name << "is not a Uint32_t" << std::endl;
res=0;
err = true;
}
else
Expand Down
70 changes: 44 additions & 26 deletions src/NativeJSRenderer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,10 +225,8 @@ void NativeJSRenderer::setEnvForConsoleMode(ModuleSettings& moduleSettings)

uint32_t NativeJSRenderer::createApplicationIdentifier()
{
static uint32_t id = 1;
uint32_t ret = id;
id++;
return ret;
static std::atomic<uint32_t> id{1};
return id++;
}

uint32_t NativeJSRenderer::createApplication(ModuleSettings& moduleSettings, std::string userAgent)
Expand Down Expand Up @@ -292,6 +290,7 @@ bool NativeJSRenderer::terminateApplication(uint32_t id)
return true;
}

// REQUIRES: Caller must hold mUserMutex
void NativeJSRenderer::createApplicationInternal(ApplicationRequest& appRequest)
{
double startTime = getTimeInMilliSec();
Expand Down Expand Up @@ -330,9 +329,9 @@ void NativeJSRenderer::createApplicationInternal(ApplicationRequest& appRequest)
context->setCreateApplicationEndTime(endTime, id);

mContextMap[id].context=context;
mUserMutex.unlock();
}
Comment thread
Sid2001-maker marked this conversation as resolved.

// REQUIRES: Caller must hold mUserMutex
void NativeJSRenderer::runApplicationInternal(ApplicationRequest& appRequest)
{
uint32_t id = appRequest.mId;
Expand Down Expand Up @@ -365,7 +364,7 @@ void NativeJSRenderer::runApplicationInternal(ApplicationRequest& appRequest)
NativeJSLogger::log(INFO, "Adding the window location: %s to js file\n", window.str().c_str());
context->runScript(window.str().c_str(),true, url, nullptr, true);
}
NativeJSLogger::log(INFO, "nativeJS application thunder execution url: %s, result: %d\n", url.c_str(), ret ? 1 : 0);
NativeJSLogger::log(INFO, "nativeJS application thunder execution url: %s\n", url.c_str());
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log message on line 367 was changed to log only the URL without the result. However, the very next line (369) logs the result anyway with the same information. This creates redundant logging where the URL is logged twice - once on line 367 without the result, and then again on line 369 with the result. Consider removing line 367 entirely since line 369 provides all the necessary information.

Suggested change
NativeJSLogger::log(INFO, "nativeJS application thunder execution url: %s\n", url.c_str());

Copilot uses AI. Check for mistakes.
ret = context->runScript(chunk.contentsBuffer, true, url, nullptr, true);
NativeJSLogger::log(INFO, "nativeJS application execution result: %d\n", ret ? 1 : 0);
double duration = context->getExecutionDuration();
Expand Down Expand Up @@ -398,6 +397,7 @@ void NativeJSRenderer::runApplicationInternal(ApplicationRequest& appRequest)
}
}

// REQUIRES: Caller must hold mUserMutex
void NativeJSRenderer::runJavaScriptInternal(ApplicationRequest& appRequest)
{
uint32_t id = appRequest.mId;
Expand Down Expand Up @@ -427,6 +427,7 @@ void NativeJSRenderer::runJavaScriptInternal(ApplicationRequest& appRequest)
}
}

// REQUIRES: Caller must hold mUserMutex
void NativeJSRenderer::terminateApplicationInternal(ApplicationRequest& AppRequest)
{
uint32_t id = AppRequest.mId;
Expand All @@ -445,7 +446,7 @@ void NativeJSRenderer::terminateApplicationInternal(ApplicationRequest& AppReque

else
{
NativeJSLogger::log(ERROR, "Unable to find application with id: %d and url: %s\n", id, mContextMap[id].url);
NativeJSLogger::log(ERROR, "Unable to find application with id: %d\n", id);
return ;
}

Expand All @@ -468,11 +469,10 @@ void NativeJSRenderer::run()
{
while(mRunning)
{
uint32_t id;
mUserMutex.lock();
if (mConsoleMode) {
processDevConsoleRequests();
}
mUserMutex.lock();
for (int i=0; i<gPendingRequests.size(); i++)
{

Expand Down Expand Up @@ -506,10 +506,13 @@ void NativeJSRenderer::run()
if(!mTestFileName.empty())
{
ModuleSettings settings;
uint32_t id = createApplicationIdentifier();
Comment thread
Sid2001-maker marked this conversation as resolved.
settings.enableJSDOM = mEnableTestFileDOMSupport;
ApplicationRequest appRequest(id, RUN, mTestFileName, settings.enableHttp, settings.enableXHR, settings.enableWebSocket, settings.enableWebSocketEnhanced, settings.enableFetch, settings.enableJSDOM, settings.enableWindow, settings.enablePlayer);
mUserMutex.lock();
NativeJSRenderer::createApplicationInternal(appRequest);
NativeJSRenderer::runApplicationInternal(appRequest);
Comment on lines 511 to 514
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ApplicationRequest created here uses the RUN type but calls both createApplicationInternal and runApplicationInternal. This is inconsistent with the normal flow where a CREATE type request would be used for createApplicationInternal. This means the appRequest has type RUN when it's passed to createApplicationInternal, which might cause issues if createApplicationInternal logic depends on the request type.

Suggested change
ApplicationRequest appRequest(id, RUN, mTestFileName, settings.enableHttp, settings.enableXHR, settings.enableWebSocket, settings.enableWebSocketEnhanced, settings.enableFetch, settings.enableJSDOM, settings.enableWindow, settings.enablePlayer);
mUserMutex.lock();
NativeJSRenderer::createApplicationInternal(appRequest);
NativeJSRenderer::runApplicationInternal(appRequest);
ApplicationRequest createRequest(id, CREATE, mTestFileName, settings.enableHttp, settings.enableXHR, settings.enableWebSocket, settings.enableWebSocketEnhanced, settings.enableFetch, settings.enableJSDOM, settings.enableWindow, settings.enablePlayer);
ApplicationRequest runRequest(id, RUN, mTestFileName, settings.enableHttp, settings.enableXHR, settings.enableWebSocket, settings.enableWebSocketEnhanced, settings.enableFetch, settings.enableJSDOM, settings.enableWindow, settings.enablePlayer);
mUserMutex.lock();
NativeJSRenderer::createApplicationInternal(createRequest);
NativeJSRenderer::runApplicationInternal(runRequest);

Copilot uses AI. Check for mistakes.
mUserMutex.unlock();
mTestFileName = "";
}

Expand All @@ -531,27 +534,30 @@ void NativeJSRenderer::run()

void NativeJSRenderer::processDevConsoleRequests()
{
std::deque<std::string> localQueue;

// Move items from shared queue to local queue while holding the lock
mConsoleState->inputMutex.lock();

if (mConsoleState->codeToExecute.empty()) {
mConsoleState->inputMutex.unlock();
return;
}
localQueue.swap(mConsoleState->codeToExecute);
mConsoleState->inputMutex.unlock();
Comment thread
Sid2001-maker marked this conversation as resolved.
Comment thread
Sid2001-maker marked this conversation as resolved.

std::lock_guard<std::mutex> lockg(mConsoleState->isProcessing_cv_m);
bool dataProcessed = false;

for (; !mConsoleState->codeToExecute.empty(); mConsoleState->codeToExecute.pop_front()) {
bool ret = mConsoleState->consoleContext->runScript(mConsoleState->codeToExecute.front().c_str(), false);
// Process items from local queue (no race condition)
for (const auto& code : localQueue) {
bool ret = mConsoleState->consoleContext->runScript(code.c_str(), false);
dataProcessed = true;
}

if (dataProcessed) {
mConsoleState->isProcessing = false;
mConsoleState->isProcessing_cv.notify_one();
}

mConsoleState->inputMutex.unlock();
}

std::atomic_bool NativeJSRenderer::consoleLoop = true;
Expand Down Expand Up @@ -620,18 +626,30 @@ bool NativeJSRenderer::downloadFile(std::string& url, MemoryStruct& chunk)
curl = curl_easy_init();
if (curl)
{
curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1);
curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, HeaderCallback);
curl_easy_setopt(curl, CURLOPT_HEADERDATA, (void *)&chunk);
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, WriteMemoryCallback);
curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)&chunk);
curl_easy_setopt(curl, CURLOPT_TIMEOUT, 30);
curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1L);
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2);
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, true);
curl_easy_setopt(curl, CURLOPT_USERAGENT, "libcurl-agent/1.0");
curl_easy_setopt(curl, CURLOPT_PROXY, "");
// Helper lambda to check curl_easy_setopt results
auto setOptWithCheck = [&curl](CURLoption option, auto value, const char* optionName) -> bool {
CURLcode res = curl_easy_setopt(curl, option, value);
if (res != CURLE_OK) {
NativeJSLogger::log(ERROR, "Failed to set %s: %s\n", optionName, curl_easy_strerror(res));
curl_easy_cleanup(curl);
curl = nullptr;
return false;
}
return true;
};
Comment on lines +630 to +639
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same curl handle cleanup issue exists here as in jsc_lib.cpp. The lambda captures curl by reference and calls curl_easy_cleanup, but doesn't set the pointer to nullptr. While this doesn't cause a double-free due to immediate return, consider setting curl to nullptr after cleanup for defensive programming consistency.

Copilot uses AI. Check for mistakes.
Comment on lines +629 to +639
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lambda captures curl by reference and then sets it to nullptr inside the lambda when an error occurs. This creates a problem because after setting curl to nullptr, subsequent calls to setOptWithCheck will still use the captured reference which is now nullptr, leading to a crash. After the first failure, curl_easy_cleanup is called with a valid handle and curl is set to nullptr, but the reference 'curl' in the outer scope becomes nullptr. This means if curl_easy_perform is reached later (though it won't be due to early returns), it would try to use a null pointer.

Consider either: 1) Not setting curl to nullptr in the lambda, or 2) Check if curl is nullptr before each subsequent call.

Copilot uses AI. Check for mistakes.

if (!setOptWithCheck(CURLOPT_URL, url.c_str(), "CURLOPT_URL")) return ret;
if (!setOptWithCheck(CURLOPT_FOLLOWLOCATION, 1L, "CURLOPT_FOLLOWLOCATION")) return ret;
if (!setOptWithCheck(CURLOPT_HEADERFUNCTION, HeaderCallback, "CURLOPT_HEADERFUNCTION")) return ret;
if (!setOptWithCheck(CURLOPT_HEADERDATA, (void *)&chunk, "CURLOPT_HEADERDATA")) return ret;
if (!setOptWithCheck(CURLOPT_WRITEFUNCTION, WriteMemoryCallback, "CURLOPT_WRITEFUNCTION")) return ret;
if (!setOptWithCheck(CURLOPT_WRITEDATA, (void *)&chunk, "CURLOPT_WRITEDATA")) return ret;
if (!setOptWithCheck(CURLOPT_TIMEOUT, 30L, "CURLOPT_TIMEOUT")) return ret;
if (!setOptWithCheck(CURLOPT_NOSIGNAL, 1L, "CURLOPT_NOSIGNAL")) return ret;
if (!setOptWithCheck(CURLOPT_SSL_VERIFYHOST, 2L, "CURLOPT_SSL_VERIFYHOST")) return ret;
if (!setOptWithCheck(CURLOPT_SSL_VERIFYPEER, 1L, "CURLOPT_SSL_VERIFYPEER")) return ret;
if (!setOptWithCheck(CURLOPT_USERAGENT, "libcurl-agent/1.0", "CURLOPT_USERAGENT")) return ret;
if (!setOptWithCheck(CURLOPT_PROXY, "", "CURLOPT_PROXY")) return ret;


//curl_easy_setopt(curl, CURLOPT_WRITEDATA, fp);
Expand Down
9 changes: 8 additions & 1 deletion src/jsc/JavaScriptContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,13 @@ if (mModuleSettings.enablePlayer)
gTopLevelContext = nullptr;
}
mPriv->releaseAllProtected();

if (mNetworkMetricsData)
{
delete mNetworkMetricsData;
mNetworkMetricsData = nullptr;
}

JSGlobalContextRelease(mContext);
JSContextGroupRelease(mContextGroup);
rtLogInfo("%s end", __FUNCTION__);
Expand Down Expand Up @@ -394,7 +401,7 @@ if (mModuleSettings.enablePlayer)
gAAMPJSBindings = new AAMPJSBindings();
loadAAMPJSBindingsLib();
}
if (gAAMPJSBindings->fnLoadJS)
if (gAAMPJSBindings && gAAMPJSBindings->fnLoadJS)
{
gAAMPJSBindings->fnLoadJS(mContext);
}
Expand Down
2 changes: 1 addition & 1 deletion src/jsc/JavaScriptEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ bool JavaScriptEngine::initialize()
if (garbageCollectInterval)
{
garbageCollectIntervalValue = atof(garbageCollectInterval);
NativeJSLogger::log(INFO, "garbage collection interval value: %d\n", garbageCollectIntervalValue);
NativeJSLogger::log(INFO, "garbage collection interval value: %f\n", garbageCollectIntervalValue);
}
mGarbageCollectionTag = installTimeout(garbageCollectIntervalValue, true,
[engine] () mutable {
Expand Down
Loading
Loading