* Patch to make logging thread-frinedly
@ 2021-09-08 20:23 Md. Enzam Hossain via SoX-devel
0 siblings, 0 replies; only message in thread
From: Md. Enzam Hossain via SoX-devel @ 2021-09-08 20:23 UTC (permalink / raw)
To: sox-devel; +Cc: Md. Enzam Hossain
[-- Attachment #1.1: Type: text/plain, Size: 8673 bytes --]
The logging functions in SoX like lsx_debug, lsx_report, etc. write to the
global struct sox_globals before calling the actual print functions.
Example macro:
#define lsx_report sox_globals.subsystem=__FILE__,lsx_report_impl
This can create *race conditions* if run parallely.
For example, two lsx_report calls are called parallely.
lsx_report call 1 updates sox_globals.subsystem.
lsx_report call 2 updates sox_globals.subsystem.
lsx_report call 1 prints, but with incorrect subsystem of call 2.
lsx_report call 2 prints with the correct subsystem.
I am attaching a patch file to make the calls thread-friendly.
Please let me know if the patch makes sense.
--
Regards,
Enzam (enzam@ <http://who/enzam>)
*Inlined patch (also attached as file):*
From 3be8226c8ca2b4f29b6927583641cb08ef39dddf Mon Sep 17 00:00:00 2001
From: "Md. Enzam Hossain" <enzam@google.com>
Date: Wed, 8 Sep 2021 12:03:02 -0700
Subject: [PATCH] Make logging thread friendly.
If multiple threads tries to log together, the subsystem may get
overwritten by each other.
Remove the dependency on the global parameter.
---
src/libsox.c | 4 ++--
src/mp3.c | 15 +++------------
src/sox.h | 32 ++++++++++++++++++--------------
src/sox_i.h | 16 ++++++++--------
4 files changed, 31 insertions(+), 36 deletions(-)
diff --git a/src/libsox.c b/src/libsox.c
index 8e9ebad6..6fc0ec41 100644
--- a/src/libsox.c
+++ b/src/libsox.c
@@ -195,11 +195,11 @@ size_t sox_basename(char * base_buffer, size_t
base_buffer_len, const char * fil
}
#define SOX_MESSAGE_FUNCTION(name,level) \
-void name(char const * fmt, ...) { \
+void name(char const * filename, char const * fmt, ...) { \
va_list ap; \
va_start(ap, fmt); \
if (sox_globals.output_message_handler) \
-
(*sox_globals.output_message_handler)(level,sox_globals.subsystem,fmt,ap);
\
+ (*sox_globals.output_message_handler)(level,filename,fmt,ap); \
va_end(ap); \
}
diff --git a/src/mp3.c b/src/mp3.c
index 1fd144d1..552ab0db 100644
--- a/src/mp3.c
+++ b/src/mp3.c
@@ -680,26 +680,17 @@ static int startread(sox_format_t * ft)
static void errorf(const char* fmt, va_list va)
{
- sox_globals.subsystem=__FILE__;
- if (sox_globals.output_message_handler)
- (*sox_globals.output_message_handler)(1,sox_globals.subsystem,fmt,va);
- return;
+ lsx_fail(fmt, va);
}
static void debugf(const char* fmt, va_list va)
{
- sox_globals.subsystem=__FILE__;
- if (sox_globals.output_message_handler)
- (*sox_globals.output_message_handler)(4,sox_globals.subsystem,fmt,va);
- return;
+ lsx_debug(fmt, va);
}
static void msgf(const char* fmt, va_list va)
{
- sox_globals.subsystem=__FILE__;
- if (sox_globals.output_message_handler)
- (*sox_globals.output_message_handler)(3,sox_globals.subsystem,fmt,va);
- return;
+ lsx_report(fmt, va);
}
/* These functions are considered optional. If they aren't present in the
diff --git a/src/sox.h b/src/sox.h
index bac50354..b9cd9754 100644
--- a/src/sox.h
+++ b/src/sox.h
@@ -79,14 +79,14 @@ the variable being unused (especially in
macro-generated code).
/**
Plugins API:
-LSX_PRINTF12: Attribute applied to a function to indicate that it requires
-a printf-style format string for arg1 and that printf parameters start at
-arg2.
+LSX_PRINTF23: Attribute applied to a function to indicate that it requires
+a printf-style format string for arg2 and that printf parameters start at
+arg3.
*/
#ifdef __GNUC__
-#define LSX_PRINTF12 __attribute__ ((format (printf, 1, 2))) /* Function
has printf-style arguments. */
+#define LSX_PRINTF23 __attribute__ ((format (printf, 2, 3))) /* Function
has printf-style arguments. */
#else
-#define LSX_PRINTF12 /* Function has printf-style arguments. */
+#define LSX_PRINTF23 /* Function has printf-style arguments. */
#endif
/**
@@ -1330,7 +1330,7 @@ typedef struct sox_globals_t {
char const * stdin_in_use_by; /**< Private: tracks the name of the
handler currently using stdin */
char const * stdout_in_use_by; /**< Private: tracks the name of the
handler currently using stdout */
- char const * subsystem; /**< Private: tracks the name of the
handler currently writing an output message */
+ char const * subsystem; /**< Deprecated, Private: tracks the name
of the handler currently writing an output message */
char * tmp_path; /**< Private: client-configured path to
use for temporary files */
sox_bool use_magic; /**< Private: true if client has
requested use of 'magic' file-type detection */
sox_bool use_threads; /**< Private: true if client has
requested parallel effects processing */
@@ -2273,9 +2273,10 @@ Print a fatal error in libSoX.
void
LSX_API
lsx_fail_impl(
+ LSX_PARAM_IN_Z char const * filename, /**< Source code __FILE__ from
which message originates. */
LSX_PARAM_IN_PRINTF char const * fmt, /**< printf-style format string.
*/
...)
- LSX_PRINTF12;
+ LSX_PRINTF23;
/**
Plugins API:
@@ -2284,9 +2285,10 @@ Print a warning in libSoX.
void
LSX_API
lsx_warn_impl(
+ LSX_PARAM_IN_Z char const * filename, /**< Source code __FILE__ from
which message originates. */
LSX_PARAM_IN_PRINTF char const * fmt, /**< printf-style format string.
*/
...)
- LSX_PRINTF12;
+ LSX_PRINTF23;
/**
Plugins API:
@@ -2295,9 +2297,10 @@ Print an informational message in libSoX.
void
LSX_API
lsx_report_impl(
+ LSX_PARAM_IN_Z char const * filename, /**< Source code __FILE__ from
which message originates. */
LSX_PARAM_IN_PRINTF char const * fmt, /**< printf-style format string.
*/
...)
- LSX_PRINTF12;
+ LSX_PRINTF23;
/**
Plugins API:
@@ -2306,33 +2309,34 @@ Print a debug message in libSoX.
void
LSX_API
lsx_debug_impl(
+ LSX_PARAM_IN_Z char const * filename, /**< Source code __FILE__ from
which message originates. */
LSX_PARAM_IN_PRINTF char const * fmt, /**< printf-style format string.
*/
...)
- LSX_PRINTF12;
+ LSX_PRINTF23;
/**
Plugins API:
Report a fatal error in libSoX; printf-style arguments must follow.
*/
-#define lsx_fail sox_get_globals()->subsystem=__FILE__,lsx_fail_impl
+#define lsx_fail(...) lsx_fail_impl(__FILE__, __VA_ARGS__)
/**
Plugins API:
Report a warning in libSoX; printf-style arguments must follow.
*/
-#define lsx_warn sox_get_globals()->subsystem=__FILE__,lsx_warn_impl
+#define lsx_warn(...) lsx_warn_impl(__FILE__, __VA_ARGS__)
/**
Plugins API:
Report an informational message in libSoX; printf-style arguments must
follow.
*/
-#define lsx_report
sox_get_globals()->subsystem=__FILE__,lsx_report_impl
+#define lsx_report(...) lsx_report_impl(__FILE__, __VA_ARGS__)
/**
Plugins API:
Report a debug message in libSoX; printf-style arguments must follow.
*/
-#define lsx_debug sox_get_globals()->subsystem=__FILE__,lsx_debug_impl
+#define lsx_debug(...) lsx_debug_impl(__FILE__, __VA_ARGS__)
/**
Plugins API:
diff --git a/src/sox_i.h b/src/sox_i.h
index c8552f97..c843556c 100644
--- a/src/sox_i.h
+++ b/src/sox_i.h
@@ -31,10 +31,10 @@
#undef lsx_fail
#undef lsx_report
#undef lsx_warn
-#define lsx_debug sox_globals.subsystem=effp->handler.name,lsx_debug_impl
-#define lsx_fail sox_globals.subsystem=effp->handler.name,lsx_fail_impl
-#define lsx_report sox_globals.subsystem=effp->handler.name,lsx_report_impl
-#define lsx_warn sox_globals.subsystem=effp->handler.name,lsx_warn_impl
+#define lsx_debug(...) lsx_debug_impl(effp->handler.name, __VA_ARGS__)
+#define lsx_fail(...) lsx_fail_impl(effp->handler.name, __VA_ARGS__)
+#define lsx_report(...) lsx_report_impl(effp->handler.name, __VA_ARGS__)
+#define lsx_warn(...) lsx_warn_impl(effp->handler.name, __VA_ARGS__)
#endif
#define RANQD1 ranqd1(sox_globals.ranqd1)
@@ -56,11 +56,11 @@ assert_static(sizeof(off_t) == _FILE_OFFSET_BITS >> 3,
OFF_T_BUILD_PROBLEM);
FILE * lsx_tmpfile(void);
-void lsx_debug_more_impl(char const * fmt, ...) LSX_PRINTF12;
-void lsx_debug_most_impl(char const * fmt, ...) LSX_PRINTF12;
+void lsx_debug_more_impl(LSX_PARAM_IN_Z char const * filename,
LSX_PARAM_IN_PRINTF char const * fmt, ...) LSX_PRINTF23;
+void lsx_debug_most_impl(LSX_PARAM_IN_Z char const * filename,
LSX_PARAM_IN_PRINTF char const * fmt, ...) LSX_PRINTF23;
-#define lsx_debug_more
sox_get_globals()->subsystem=__FILE__,lsx_debug_more_impl
-#define lsx_debug_most
sox_get_globals()->subsystem=__FILE__,lsx_debug_most_impl
+#define lsx_debug_more(...) lsx_debug_more_impl(__FILE__, __VA_ARGS__)
+#define lsx_debug_most(...) lsx_debug_most_impl(__FILE__, __VA_ARGS__)
/* Digitise one cycle of a wave and store it as
* a table of samples of a specified data-type.
--
2.33.0.153.gba50c8fa24-goog
[-- Attachment #1.2: Type: text/html, Size: 10584 bytes --]
[-- Attachment #2: 0001-Make-logging-thread-friendly.patch --]
[-- Type: text/x-patch, Size: 7949 bytes --]
From 3be8226c8ca2b4f29b6927583641cb08ef39dddf Mon Sep 17 00:00:00 2001
From: "Md. Enzam Hossain" <enzam@google.com>
Date: Wed, 8 Sep 2021 12:03:02 -0700
Subject: [PATCH] Make logging thread friendly.
If multiple threads tries to log together, the subsystem may get
overwritten by each other.
Remove the dependency on the global parameter.
---
src/libsox.c | 4 ++--
src/mp3.c | 15 +++------------
src/sox.h | 32 ++++++++++++++++++--------------
src/sox_i.h | 16 ++++++++--------
4 files changed, 31 insertions(+), 36 deletions(-)
diff --git a/src/libsox.c b/src/libsox.c
index 8e9ebad6..6fc0ec41 100644
--- a/src/libsox.c
+++ b/src/libsox.c
@@ -195,11 +195,11 @@ size_t sox_basename(char * base_buffer, size_t base_buffer_len, const char * fil
}
#define SOX_MESSAGE_FUNCTION(name,level) \
-void name(char const * fmt, ...) { \
+void name(char const * filename, char const * fmt, ...) { \
va_list ap; \
va_start(ap, fmt); \
if (sox_globals.output_message_handler) \
- (*sox_globals.output_message_handler)(level,sox_globals.subsystem,fmt,ap); \
+ (*sox_globals.output_message_handler)(level,filename,fmt,ap); \
va_end(ap); \
}
diff --git a/src/mp3.c b/src/mp3.c
index 1fd144d1..552ab0db 100644
--- a/src/mp3.c
+++ b/src/mp3.c
@@ -680,26 +680,17 @@ static int startread(sox_format_t * ft)
static void errorf(const char* fmt, va_list va)
{
- sox_globals.subsystem=__FILE__;
- if (sox_globals.output_message_handler)
- (*sox_globals.output_message_handler)(1,sox_globals.subsystem,fmt,va);
- return;
+ lsx_fail(fmt, va);
}
static void debugf(const char* fmt, va_list va)
{
- sox_globals.subsystem=__FILE__;
- if (sox_globals.output_message_handler)
- (*sox_globals.output_message_handler)(4,sox_globals.subsystem,fmt,va);
- return;
+ lsx_debug(fmt, va);
}
static void msgf(const char* fmt, va_list va)
{
- sox_globals.subsystem=__FILE__;
- if (sox_globals.output_message_handler)
- (*sox_globals.output_message_handler)(3,sox_globals.subsystem,fmt,va);
- return;
+ lsx_report(fmt, va);
}
/* These functions are considered optional. If they aren't present in the
diff --git a/src/sox.h b/src/sox.h
index bac50354..b9cd9754 100644
--- a/src/sox.h
+++ b/src/sox.h
@@ -79,14 +79,14 @@ the variable being unused (especially in macro-generated code).
/**
Plugins API:
-LSX_PRINTF12: Attribute applied to a function to indicate that it requires
-a printf-style format string for arg1 and that printf parameters start at
-arg2.
+LSX_PRINTF23: Attribute applied to a function to indicate that it requires
+a printf-style format string for arg2 and that printf parameters start at
+arg3.
*/
#ifdef __GNUC__
-#define LSX_PRINTF12 __attribute__ ((format (printf, 1, 2))) /* Function has printf-style arguments. */
+#define LSX_PRINTF23 __attribute__ ((format (printf, 2, 3))) /* Function has printf-style arguments. */
#else
-#define LSX_PRINTF12 /* Function has printf-style arguments. */
+#define LSX_PRINTF23 /* Function has printf-style arguments. */
#endif
/**
@@ -1330,7 +1330,7 @@ typedef struct sox_globals_t {
char const * stdin_in_use_by; /**< Private: tracks the name of the handler currently using stdin */
char const * stdout_in_use_by; /**< Private: tracks the name of the handler currently using stdout */
- char const * subsystem; /**< Private: tracks the name of the handler currently writing an output message */
+ char const * subsystem; /**< Deprecated, Private: tracks the name of the handler currently writing an output message */
char * tmp_path; /**< Private: client-configured path to use for temporary files */
sox_bool use_magic; /**< Private: true if client has requested use of 'magic' file-type detection */
sox_bool use_threads; /**< Private: true if client has requested parallel effects processing */
@@ -2273,9 +2273,10 @@ Print a fatal error in libSoX.
void
LSX_API
lsx_fail_impl(
+ LSX_PARAM_IN_Z char const * filename, /**< Source code __FILE__ from which message originates. */
LSX_PARAM_IN_PRINTF char const * fmt, /**< printf-style format string. */
...)
- LSX_PRINTF12;
+ LSX_PRINTF23;
/**
Plugins API:
@@ -2284,9 +2285,10 @@ Print a warning in libSoX.
void
LSX_API
lsx_warn_impl(
+ LSX_PARAM_IN_Z char const * filename, /**< Source code __FILE__ from which message originates. */
LSX_PARAM_IN_PRINTF char const * fmt, /**< printf-style format string. */
...)
- LSX_PRINTF12;
+ LSX_PRINTF23;
/**
Plugins API:
@@ -2295,9 +2297,10 @@ Print an informational message in libSoX.
void
LSX_API
lsx_report_impl(
+ LSX_PARAM_IN_Z char const * filename, /**< Source code __FILE__ from which message originates. */
LSX_PARAM_IN_PRINTF char const * fmt, /**< printf-style format string. */
...)
- LSX_PRINTF12;
+ LSX_PRINTF23;
/**
Plugins API:
@@ -2306,33 +2309,34 @@ Print a debug message in libSoX.
void
LSX_API
lsx_debug_impl(
+ LSX_PARAM_IN_Z char const * filename, /**< Source code __FILE__ from which message originates. */
LSX_PARAM_IN_PRINTF char const * fmt, /**< printf-style format string. */
...)
- LSX_PRINTF12;
+ LSX_PRINTF23;
/**
Plugins API:
Report a fatal error in libSoX; printf-style arguments must follow.
*/
-#define lsx_fail sox_get_globals()->subsystem=__FILE__,lsx_fail_impl
+#define lsx_fail(...) lsx_fail_impl(__FILE__, __VA_ARGS__)
/**
Plugins API:
Report a warning in libSoX; printf-style arguments must follow.
*/
-#define lsx_warn sox_get_globals()->subsystem=__FILE__,lsx_warn_impl
+#define lsx_warn(...) lsx_warn_impl(__FILE__, __VA_ARGS__)
/**
Plugins API:
Report an informational message in libSoX; printf-style arguments must follow.
*/
-#define lsx_report sox_get_globals()->subsystem=__FILE__,lsx_report_impl
+#define lsx_report(...) lsx_report_impl(__FILE__, __VA_ARGS__)
/**
Plugins API:
Report a debug message in libSoX; printf-style arguments must follow.
*/
-#define lsx_debug sox_get_globals()->subsystem=__FILE__,lsx_debug_impl
+#define lsx_debug(...) lsx_debug_impl(__FILE__, __VA_ARGS__)
/**
Plugins API:
diff --git a/src/sox_i.h b/src/sox_i.h
index c8552f97..c843556c 100644
--- a/src/sox_i.h
+++ b/src/sox_i.h
@@ -31,10 +31,10 @@
#undef lsx_fail
#undef lsx_report
#undef lsx_warn
-#define lsx_debug sox_globals.subsystem=effp->handler.name,lsx_debug_impl
-#define lsx_fail sox_globals.subsystem=effp->handler.name,lsx_fail_impl
-#define lsx_report sox_globals.subsystem=effp->handler.name,lsx_report_impl
-#define lsx_warn sox_globals.subsystem=effp->handler.name,lsx_warn_impl
+#define lsx_debug(...) lsx_debug_impl(effp->handler.name, __VA_ARGS__)
+#define lsx_fail(...) lsx_fail_impl(effp->handler.name, __VA_ARGS__)
+#define lsx_report(...) lsx_report_impl(effp->handler.name, __VA_ARGS__)
+#define lsx_warn(...) lsx_warn_impl(effp->handler.name, __VA_ARGS__)
#endif
#define RANQD1 ranqd1(sox_globals.ranqd1)
@@ -56,11 +56,11 @@ assert_static(sizeof(off_t) == _FILE_OFFSET_BITS >> 3, OFF_T_BUILD_PROBLEM);
FILE * lsx_tmpfile(void);
-void lsx_debug_more_impl(char const * fmt, ...) LSX_PRINTF12;
-void lsx_debug_most_impl(char const * fmt, ...) LSX_PRINTF12;
+void lsx_debug_more_impl(LSX_PARAM_IN_Z char const * filename, LSX_PARAM_IN_PRINTF char const * fmt, ...) LSX_PRINTF23;
+void lsx_debug_most_impl(LSX_PARAM_IN_Z char const * filename, LSX_PARAM_IN_PRINTF char const * fmt, ...) LSX_PRINTF23;
-#define lsx_debug_more sox_get_globals()->subsystem=__FILE__,lsx_debug_more_impl
-#define lsx_debug_most sox_get_globals()->subsystem=__FILE__,lsx_debug_most_impl
+#define lsx_debug_more(...) lsx_debug_more_impl(__FILE__, __VA_ARGS__)
+#define lsx_debug_most(...) lsx_debug_most_impl(__FILE__, __VA_ARGS__)
/* Digitise one cycle of a wave and store it as
* a table of samples of a specified data-type.
--
2.33.0.153.gba50c8fa24-goog
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
[-- Attachment #4: Type: text/plain, Size: 158 bytes --]
_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2021-09-08 21:23 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-08 20:23 Patch to make logging thread-frinedly Md. Enzam Hossain via SoX-devel
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).