Date: Wed, 24 Jan 2001 19:48:25 -0800 From: Mike Smith <msmith@freebsd.org> To: Kevin Day <toasty@temphost.dragondata.com> Cc: hackers@freebsd.org, kevin@stileproject.com Subject: Re: Pthreads with sprintf/dtoa Message-ID: <200101250348.f0P3mPI02756@mass.dis.org> In-Reply-To: Your message of "Wed, 24 Jan 2001 18:29:58 CST." <200101250029.SAA23574@temphost.dragondata.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multipart MIME message.
--==_Exmh_-7024949920
Content-Type: text/plain; charset=us-ascii
>
>
> After upgrading to FreeBSD 4.2(from 4.1) and MySQL 3.23.32 (from 3.22.32), I
> kept seeing mysqld crashes after a few minutes of heavy load. I traced it
> down to one rather situation. Every time it crashed, I was getting a
> segfault inside __dtoa (which was called by sprintf). If I looked at other
> threads, I'd always see another thread inside __dtoa at that point too.
>
> I went back to 3.22.32, and it still happened, so my current guess is
> something between 4.1-RELEASE and 4.2-RELEASE.
>
> I'm really highly ignorant of pthreads, so I have no idea if they(myslqd) is
> doing something wrong thread-wise, or something got broken so that it's no
> longer thread safe inside sprintf or dtoa.
>
> Can someone cluefull point me in the right direction?
Sure.
static Bigint *result;
static int result_k;
__dtoa has static locals. Bad function, no biscuit.
I think that it does this in an attempt to avoid malloc thrash if it's
called frequently; only resizing the result buffer if it actually needs a
larger one.
Making this work "right" will be a bit ugly, since you'll need to fix
__dtoa to always return a buffer allocated with malloc, and then teach
vfprintf how to deal with a volatile string. The handling of the result
buffer in __dtoa is truly evil, and the author should most definitely
have been shot.
Attached is an (untested) patch. The sizing of the return buffer in
__dtoa is a guess, based on trying to work out what the original code was
doing, and may be off-by-one.
--==_Exmh_-7024949920
Content-Type: application/x-patch ; name="dtoa.patch"
Content-Description: dtoa.patch
Content-Disposition: attachment; filename="dtoa.patch"
Index: stdio/vfprintf.c
===================================================================
RCS file: /local0/src/src/lib/libc/stdio/vfprintf.c,v
retrieving revision 1.23
diff -u -r1.23 vfprintf.c
--- stdio/vfprintf.c 2001/01/06 20:48:00 1.23
+++ stdio/vfprintf.c 2001/01/25 03:25:28
@@ -287,6 +287,7 @@
#define SHORTINT 0x040 /* short integer */
#define ZEROPAD 0x080 /* zero (as opposed to blank) pad */
#define FPT 0x100 /* Floating point number */
+#define MUSTFREE 0x200 /* sp must be freed */
int
vfprintf(fp, fmt0, ap)
FILE *fp;
@@ -610,6 +611,7 @@
flags |= FPT;
cp = cvt(_double, prec, flags, &softsign,
&expt, ch, &ndig);
+ flags |= MUSTFREE;
if (ch == 'g' || ch == 'G') {
if (expt <= -4 || expt > prec)
ch = (ch == 'g') ? 'e' : 'E';
@@ -861,6 +863,10 @@
ret += prsize;
FLUSH(); /* copy out the I/O vectors */
+
+ /* free cp if it was allocated elsewhere */
+ if (flags & MUSTFREE)
+ free(cp);
}
done:
FLUSH();
Index: stdlib/strtod.c
===================================================================
RCS file: /local0/src/src/lib/libc/stdlib/strtod.c,v
retrieving revision 1.3
diff -u -r1.3 strtod.c
--- stdlib/strtod.c 1996/07/12 18:55:22 1.3
+++ stdlib/strtod.c 2001/01/25 03:43:08
@@ -1890,16 +1890,7 @@
Bigint *b, *b1, *delta, *mlo, *mhi, *S;
double d2, ds, eps;
char *s, *s0;
- static Bigint *result;
- static int result_k;
-
- if (result) {
- result->k = result_k;
- result->maxwds = 1 << result_k;
- Bfree(result);
- result = 0;
- }
-
+
if (word0(d) & Sign_bit) {
/* set sign for everything, including 0's and NaNs */
*sign = 1;
@@ -1917,11 +1908,11 @@
{
/* Infinity or NaN */
*decpt = 9999;
- s =
+ s = strdup(
#ifdef IEEE_Arith
!word1(d) && !(word0(d) & 0xfffff) ? "Infinity" :
#endif
- "NaN";
+ "NaN");
if (rve)
*rve =
#ifdef IEEE_Arith
@@ -1936,7 +1927,7 @@
#endif
if (!d) {
*decpt = 1;
- s = "0";
+ s = strdup("0");
if (rve)
*rve = s + 1;
return s;
@@ -2057,11 +2048,10 @@
if (i <= 0)
i = 1;
}
- j = sizeof(unsigned long);
- for (result_k = 0; sizeof(Bigint) - sizeof(unsigned long) + j < i;
- j <<= 1) result_k++;
- result = Balloc(result_k);
- s = s0 = (char *)result;
+ /* XXX verify that i is always large enough? Add padding? */
+ s = s0 = malloc(i);
+ if (s0 == NULL)
+ return s0;
if (ilim >= 0 && ilim <= Quick_max && try_quick) {
--==_Exmh_-7024949920
Content-Type: text/plain; charset=us-ascii
... every activity meets with opposition, everyone who acts has his
rivals and unfortunately opponents also. But not because people want
to be opponents, rather because the tasks and relationships force
people to take different points of view. [Dr. Fritz Todt]
V I C T O R Y N O T V E N G E A N C E
--==_Exmh_-7024949920--
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200101250348.f0P3mPI02756>