Skip to content

Commit 77b1725

Browse files
kdhawaniyaCopilot
andcommitted
Fix data race in JavaDataViewerProxy::GetCurrentEndpoint
IDataViewer returns the endpoint by const reference, so the referent must outlive the call and must not be mutated while a caller holds it. The proxy updated a shared member under a mutex and then returned a reference to it, releasing the lock on return: two concurrent callers could read and write the same string at once, so the mutex gave no protection. Use a thread_local buffer instead, which gives each calling thread its own storage and removes the need for the lock. Behaviour is unchanged: the endpoint is still read from Java on every call, so a viewer that changes endpoints still reports the current one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 65d01fa commit 77b1725

2 files changed

Lines changed: 21 additions & 16 deletions

File tree

‎lib/jni/JavaDataViewerProxy.cpp‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -168,23 +168,31 @@ namespace MAT_NS_BEGIN
168168

169169
const std::string& JavaDataViewerProxy::GetCurrentEndpoint() const noexcept
170170
{
171-
std::lock_guard<std::mutex> lock(m_endpointMutex);
171+
// IDataViewer returns the endpoint by reference, so the referent has to outlive the
172+
// call and must not be mutated by a concurrent caller. A thread_local buffer gives
173+
// each calling thread its own storage; a shared member guarded by a mutex would not,
174+
// because the lock is released before the caller reads the reference.
175+
static thread_local std::string currentEndpoint;
176+
172177
bool attached = false;
173178
auto env = GetEnv(attached);
174-
if (env != nullptr)
179+
if (env == nullptr)
175180
{
176-
std::string endpoint;
177-
if (ReadString(env, m_getCurrentEndpoint, endpoint))
178-
{
179-
m_currentEndpoint = std::move(endpoint);
180-
}
181-
else
182-
{
183-
m_currentEndpoint.clear();
184-
}
185-
DetachIfNeeded(attached);
181+
currentEndpoint.clear();
182+
return currentEndpoint;
183+
}
184+
185+
std::string endpoint;
186+
if (ReadString(env, m_getCurrentEndpoint, endpoint))
187+
{
188+
currentEndpoint = std::move(endpoint);
186189
}
187-
return m_currentEndpoint;
190+
else
191+
{
192+
currentEndpoint.clear();
193+
}
194+
DetachIfNeeded(attached);
195+
return currentEndpoint;
188196
}
189197

190198
JNIEnv* JavaDataViewerProxy::GetEnv(bool& attached) const noexcept

‎lib/jni/JavaDataViewerProxy.hpp‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
#include <jni.h>
1111
#include <memory>
12-
#include <mutex>
1312
#include <string>
1413

1514
namespace MAT_NS_BEGIN
@@ -41,8 +40,6 @@ namespace MAT_NS_BEGIN
4140
jmethodID m_isTransmissionEnabled = nullptr;
4241
jmethodID m_getCurrentEndpoint = nullptr;
4342
std::string m_name;
44-
mutable std::mutex m_endpointMutex;
45-
mutable std::string m_currentEndpoint;
4643
};
4744

4845
} MAT_NS_END

0 commit comments

Comments
 (0)