* [binutils-gdb] gprofng: fix data race
@ 2023-07-01 15:42 Vladimir Mezentsev
0 siblings, 0 replies; only message in thread
From: Vladimir Mezentsev @ 2023-07-01 15:42 UTC (permalink / raw)
To: bfd-cvs, gdb-cvs
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=c476793d5bb9e7a0945411149128657a56d4b06c
commit c476793d5bb9e7a0945411149128657a56d4b06c
Author: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
Date: Thu Jun 29 13:11:09 2023 -0700
gprofng: fix data race
In our GUI project (https://savannah.gnu.org/projects/gprofng-gui), we use
the output of gprofng to display the data. Sometimes this data is corrupted.
gprofng/ChangeLog
2023-06-29 Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
* src/ipc.cc (ipc_doWork): Fix data race.
* src/ipcio.cc (IPCresponse::print): Fix data race.
Remove unused variables and functions.
* src/ipcio.h: Declare two variables.
* src/StringBuilder.cc (StringBuilder::write): New function.
* src/StringBuilder.h: Likewise.
Diff:
---
gprofng/src/StringBuilder.cc | 8 ++++
gprofng/src/StringBuilder.h | 1 +
gprofng/src/ipc.cc | 19 ++++-----
gprofng/src/ipcio.cc | 97 +++++++-------------------------------------
gprofng/src/ipcio.h | 2 +
5 files changed, 34 insertions(+), 93 deletions(-)
diff --git a/gprofng/src/StringBuilder.cc b/gprofng/src/StringBuilder.cc
index a806261d026..f312866bd23 100644
--- a/gprofng/src/StringBuilder.cc
+++ b/gprofng/src/StringBuilder.cc
@@ -24,6 +24,7 @@
#include <string.h>
#include <values.h>
#include <stdarg.h>
+#include <unistd.h>
#include "gp-defs.h"
#include "StringBuilder.h"
@@ -447,6 +448,13 @@ StringBuilder::toFileLn (FILE *fp)
fprintf (fp, NTXT ("%s\n"), value);
}
+void
+StringBuilder::write (int fd)
+{
+ if (count > 0)
+ ::write (fd, value, count);
+}
+
StringBuilder *
StringBuilder::sprintf (const char *fmt, ...)
{
diff --git a/gprofng/src/StringBuilder.h b/gprofng/src/StringBuilder.h
index cb7127bc137..8db90c51239 100644
--- a/gprofng/src/StringBuilder.h
+++ b/gprofng/src/StringBuilder.h
@@ -82,6 +82,7 @@ public:
char *toString ();
void toFile (FILE *fp);
void toFileLn (FILE *fp);
+ void write (int fd);
// Not in Java
StringBuilder *appendf (const char *fmt, ...) __attribute__ ((format (printf, 2, 3)));
diff --git a/gprofng/src/ipc.cc b/gprofng/src/ipc.cc
index 3cf6b8f0ecc..d0f15d3813f 100644
--- a/gprofng/src/ipc.cc
+++ b/gprofng/src/ipc.cc
@@ -189,11 +189,10 @@ sigterm_handler (int, siginfo_t *, void *)
static const char *ipc_log_name = NULL;
static const char *ipc_request_log_name = NULL;
static const char *ipc_response_log_name = NULL;
-FILE *requestLogFileP = stderr;
-FILE *responseLogFileP = stderr;
-hrtime_t begin_time;
-long long delta_time = 0;
-int ipc_delay_microsec = 0;
+static FILE *requestLogFileP = stderr;
+static FILE *responseLogFileP = stderr;
+static hrtime_t begin_time;
+static long long delta_time = 0;
void
ipc_default_log (const char *fmt, ...)
@@ -362,7 +361,7 @@ ipc_doWork (void *arg)
ipc_log ("NULL ipc command received, exiting\n");
return 0;
}
- ipc_log ("ipc: %s Req %x Ch %x\n", inp, currentRequestID, currentChannelID);
+ ipc_log ("ipc: %s Req %x Ch %x\n", inp, req->getRequestID (), req->getChannelID ());
checkCancellableOp (inp, req);
if (!strcmp (inp, "initApplication"))
{
@@ -788,7 +787,7 @@ ipc_doWork (void *arg)
int dbevindex = readInt (req);
int cmp_mode = readInt (req);
getView (dbevindex)->set_compare_mode (cmp_mode);
- writeResponseGeneric (RESPONSE_STATUS_SUCCESS, currentRequestID, currentChannelID);
+ writeResponseGeneric (RESPONSE_STATUS_SUCCESS, req->getRequestID (), req->getChannelID ());
}
else if (!strcmp (inp, "getCompareModeV2"))
{
@@ -811,7 +810,7 @@ ipc_doWork (void *arg)
int cmp_mode = readInt (req);
MetricList *mlist = readMetricListV2 (dbevindex, req);
getView (dbevindex)->reset_metric_list (mlist, cmp_mode);
- writeResponseGeneric (RESPONSE_STATUS_SUCCESS, currentRequestID, currentChannelID);
+ writeResponseGeneric (RESPONSE_STATUS_SUCCESS, req->getRequestID (), req->getChannelID ());
}
else if (!strcmp (inp, "getCurMetricsV2"))
{
@@ -2429,7 +2428,7 @@ ipc_doWork (void *arg)
dbe_archive (ids, locations);
delete ids;
destroy (locations);
- writeResponseGeneric (RESPONSE_STATUS_SUCCESS, currentRequestID, currentChannelID);
+ writeResponseGeneric (RESPONSE_STATUS_SUCCESS, req->getRequestID (), req->getChannelID ());
}
else if (strcmp (inp, "dbeSetLocations") == 0)
{
@@ -2438,7 +2437,7 @@ ipc_doWork (void *arg)
dbeSetLocations (fnames, locations);
destroy (fnames);
destroy (locations);
- writeResponseGeneric (RESPONSE_STATUS_SUCCESS, currentRequestID, currentChannelID);
+ writeResponseGeneric (RESPONSE_STATUS_SUCCESS, req->getRequestID (), req->getChannelID ());
}
else if (strcmp (inp, "dbeResolvedWith_setpath") == 0)
{
diff --git a/gprofng/src/ipcio.cc b/gprofng/src/ipcio.cc
index 9a6b7afc22d..54648cdcbcc 100644
--- a/gprofng/src/ipcio.cc
+++ b/gprofng/src/ipcio.cc
@@ -52,13 +52,6 @@ static const int L_CHAR = 8;
int currentRequestID;
int currentChannelID;
-static long maxSize;
-
-extern int cancellableChannelID;
-extern int error_flag;
-extern int ipc_delay_microsec;
-extern FILE *responseLogFileP;
-
IPCresponse *IPCresponseGlobal;
BufferPool *responseBufferPool;
@@ -624,52 +617,6 @@ IPCresponse::sendAVal (void *ptr)
}
}
-static void
-writeResponseHeader (int requestID, int responseType, int responseStatus, int nBytes)
-{
- if (responseType == RESPONSE_TYPE_HANDSHAKE)
- nBytes = IPC_VERSION_NUMBER;
- int use_write = 2;
- ipc_response_trace (TRACE_LVL_1, "ResponseHeaderBegin----- %x ---- %x ----- %x -----%x -------\n", requestID, responseType, responseStatus, nBytes);
- if (use_write)
- {
- char buf[23];
- if (use_write == 1)
- {
- int i = 0;
- snprintf (buf + i, 3, "%2x", HEADER_MARKER);
- i += 2;
- snprintf (buf + i, 9, "%8x", requestID);
- i += 8;
- snprintf (buf + i, 3, "%2x", responseType);
- i += 2;
- snprintf (buf + i, 3, "%2x", responseStatus);
- i += 2;
- snprintf (buf + i, 9, "%8x", nBytes);
- }
- else
- snprintf (buf, 23, "%02x%08x%02x%02x%08x", HEADER_MARKER, requestID,
- responseType, responseStatus, nBytes);
- buf[22] = 0;
- write (1, buf, 22);
- }
- else
- {
- cout << setfill ('0') << setw (2) << hex << HEADER_MARKER;
- cout << setfill ('0') << setw (8) << hex << requestID;
- cout << setfill ('0') << setw (2) << hex << responseType;
- cout << setfill ('0') << setw (2) << hex << responseStatus;
- cout << setfill ('0') << setw (8) << hex << nBytes;
- cout.flush ();
- }
- ipc_response_trace (TRACE_LVL_1, "----------------------------ResponseHeaderEnd\n");
- if (nBytes > maxSize)
- {
- maxSize = nBytes;
- ipc_trace ("New maxsize %ld\n", maxSize);
- }
-}
-
bool
cancelNeeded (int chID)
{
@@ -698,12 +645,6 @@ writeResponseWithHeader (int requestID, int channelID, int responseType,
responseBufferPool->recycle (os);
}
-void
-writeAckFast (int requestID)
-{
- writeResponseHeader (requestID, RESPONSE_TYPE_ACK, RESPONSE_STATUS_SUCCESS, 0);
-}
-
void
writeAck (int requestID, int channelID)
{
@@ -731,7 +672,6 @@ writeHandshake (int requestID, int channelID)
{
IPCresponse *OUTS = responseBufferPool->getNewResponse (BUFFER_SIZE_SMALL);
writeResponseWithHeader (requestID, channelID, RESPONSE_TYPE_HANDSHAKE, RESPONSE_STATUS_SUCCESS, OUTS);
- // writeResponseHeader(requestID, RESPONSE_TYPE_HANDSHAKE, RESPONSE_STATUS_SUCCESS, IPC_VERSION_NUMBER);
}
void
@@ -923,30 +863,23 @@ setProgress (int percentage, const char *proc_str)
return 0;
}
+static pthread_mutex_t responce_lock = PTHREAD_MUTEX_INITIALIZER;
+
void
IPCresponse::print (void)
{
- if (ipc_delay_microsec)
- usleep (ipc_delay_microsec);
- int stringSize = sb->length ();
- writeResponseHeader (requestID, responseType, responseStatus, stringSize);
- if (stringSize > 0)
- {
- char *s = sb->toString ();
- hrtime_t start_time = gethrtime ();
- int use_write = 1;
- if (use_write)
- write (1, s, stringSize); // write(1, sb->toString(), stringSize);
- else
- {
- cout << s;
- cout.flush ();
- }
- hrtime_t end_time = gethrtime ();
- unsigned long long time_stamp = end_time - start_time;
- ipc_response_log (TRACE_LVL_3, "ReqID %x flush time %llu nanosec \n", requestID, time_stamp);
- free (s);
- }
+ char buf[23];
+ int sz = responseType == RESPONSE_TYPE_HANDSHAKE ?
+ IPC_VERSION_NUMBER : sb->length ();
+ snprintf (buf, sizeof (buf), "%02x%08x%02x%02x%08x", HEADER_MARKER,
+ requestID, responseType, responseStatus, sz);
+ pthread_mutex_lock (&responce_lock);
+ ipc_response_trace (TRACE_LVL_1,
+ "IPCresponse: ID=%08x type=%02x status=%02x sz:%6d\n",
+ requestID, responseType, responseStatus, sz);
+ write (1, buf, 22);
+ sb->write (1);
+ pthread_mutex_unlock (&responce_lock);
}
void
@@ -974,9 +907,7 @@ readRequestHeader ()
if (requestType == REQUEST_TYPE_HANDSHAKE)
{
// write the ack directly to the wire, not through the response queue
- // writeAckFast(requestID);
writeAck (requestID, channelID);
- maxSize = 0;
writeHandshake (requestID, channelID);
ipc_request_trace (TRACE_LVL_1, "RQ: HANDSHAKE --- %x ----- %x ---- %x --- %x -RequestHeaderEnd\n", requestID, requestType, channelID, nBytes);
}
diff --git a/gprofng/src/ipcio.h b/gprofng/src/ipcio.h
index 05ff30ba34b..6c97dc7ee0c 100644
--- a/gprofng/src/ipcio.h
+++ b/gprofng/src/ipcio.h
@@ -168,6 +168,8 @@ extern int ipc_single_threaded_mode;
extern DbeThreadPool *responseThreadPool;
extern DbeThreadPool *ipcThreadPool;
extern int cancelRequestedChannelID;
+extern int cancellableChannelID;
+extern int error_flag;
void ipc_default_log (const char *fmt, ...) __attribute__ ((format (printf, 1, 2)));
void ipc_response_log (IPCTraceLevel, const char *fmt, ...) __attribute__ ((format (printf, 2, 3)));
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2023-07-01 15:42 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-01 15:42 [binutils-gdb] gprofng: fix data race Vladimir Mezentsev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).