From 0424ce9b0a4eb66b2d5287275d82e3902ac90c22 Mon Sep 17 00:00:00 2001 From: Josh Davies Date: Tue, 15 Sep 2026 14:30:52 +0100 Subject: [PATCH] fix+feat: protect global extrasymbol buffer with mutex for local poly conversion This fixes #138 by having local polynomial conversions hold a mutex for the duration, to avoid corruption due to concurrent modifications of the global extrasymbol buffer. This means that we can now also allow simultaneous use of global extrasymbols (ArgToExtraSymbol, ToPolynomial/FromPolynomial) and functions which trigger local conversion to polynomials: FactArg, FactDollar, gcd_, etc. Historically this has been forbidden in both FORM and TFORM, although FORM should not ever have had an issue. Presumably it was forbidden for both for consistency. --- check/fixes.frm | 43 +++++++++++ sources/argument.c | 13 ++-- sources/compcomm.c | 19 +---- sources/comtool.c | 8 ++- sources/declare.h | 8 ++- sources/dollar.c | 9 ++- sources/ftypes.h | 1 - sources/notation.c | 169 ++++++++++++++++++++++++++++---------------- sources/polywrap.cc | 15 +++- sources/ratio.c | 20 +++--- sources/structs.h | 4 +- sources/threads.c | 6 +- 12 files changed, 212 insertions(+), 103 deletions(-) diff --git a/check/fixes.frm b/check/fixes.frm index 3ad37168c..caef7e18a 100644 --- a/check/fixes.frm +++ b/check/fixes.frm @@ -1409,6 +1409,49 @@ assert stdout =~ exact_pattern(<<'EOF') F EOF *--#] Issue129_2 : +*--#[ Issue138_1 : +CFunction f,a,b,c,d; +Symbol x,y; +Local F = (1+f((a+b)^2,(a+b)*(c*d)))^20; +.sort +Identify f(x?,y?) = f(gcd_(x,y)); +ToPolynomial; +.sort +FromPolynomial; +Identify f(a+b) = -1; +Print; +.end +assert succeeded? +assert result("F") =~ expr("0") +*--#] Issue138_1 : +*--#[ Issue138_2 : +CFunction f,g; +Symbol x,y; +Local F = (1+f((g(x)+g(y))^2))^20; +FactArg f; +ToPolynomial; +.sort +FromPolynomial; +Identify f(g(x)+g(y),g(x)+g(y)) = -1; +Print; +.end +assert succeeded? +assert result("F") =~ expr("0") +*--#] Issue138_2 : +*--#[ Issue138_3 : +CFunction f,g; +Symbol x,y; +Local F = (1+f((g(x)+g(y))^2))^20; +FactArg f; +ArgToExtraSymbol g; +.sort +FromPolynomial; +Identify f(g(x)+g(y),g(x)+g(y)) = -1; +Print; +.end +assert succeeded? +assert result("F") =~ expr("0") +*--#] Issue138_3 : *--#[ Issue139 : * Corrupted characters in printing f(-2147483648) CF f; diff --git a/sources/argument.c b/sources/argument.c index 12a2308fe..60a5f4912 100644 --- a/sources/argument.c +++ b/sources/argument.c @@ -2078,7 +2078,7 @@ int ArgFactorize(PHEAD WORD *argin, WORD *argout) #endif WORD startebuf = cbuf[AT.ebufnum].numrhs,oldword; WORD oldsorttype = AR.SortType, numargs; - int error = 0, action = 0, i, ii, number, sign = 1; + int error = 0, action = 0, i, ii, locked = 0, number, sign = 1; *argout = 0; /* @@ -2254,9 +2254,10 @@ int ArgFactorize(PHEAD WORD *argin, WORD *argout) argextra = AT.WorkPointer; NewSort(BHEAD0); while ( t < tstop ) { - if ( LocalConvertToPoly(BHEAD t,argextra,startebuf,0) < 0 ) { + if ( LocalConvertToPoly(BHEAD t,argextra,startebuf,0,&locked) < 0 ) { error = -1; getout: + UnlockLocalPolynomial(&locked); AR.SortType = oldsorttype; TermFree(argcopy,"argcopy"); if ( argfree != argin ) TermFree(argfree,"argfree"); @@ -2350,6 +2351,7 @@ int ArgFactorize(PHEAD WORD *argin, WORD *argout) for ( i = 0; i <= *argcopy; i++ ) a[i] = argcopy[i]; } } + UnlockLocalPolynomial(&locked); /* #] step 6 : #[ step 7 : Add this one to the tables. @@ -2526,9 +2528,12 @@ WORD FindArg(PHEAD WORD *a) { int number; if ( AN.ncmod != 0 ) return(0); /* no room for mod stuff */ - number = FindTree(AT.fbufnum,a); + /* FindTree is not strictly thread safe, but AT.fbufnum is thread local */ + number = FindTree(AT.fbufnum,a,1); if ( number >= 0 ) return(number+1); - number = FindTree(AC.ffbufnum,a); + /* AC.ffbufnum is global, but currently unused. If it becomes used, we need + * to lock here! */ + number = FindTree(AC.ffbufnum,a,1); if ( number >= 0 ) return(-number-1); return(0); } diff --git a/sources/compcomm.c b/sources/compcomm.c index 4c281613b..55a117667 100644 --- a/sources/compcomm.c +++ b/sources/compcomm.c @@ -2223,11 +2223,6 @@ int CoSplitLastArg(UBYTE *s) { return(DoArgument(s,TYPESPLITLASTARG)); } */ int CoFactArg(UBYTE *s) { - if ( ( AC.topolynomialflag & TOPOLYNOMIALFLAG ) != 0 ) { - MesPrint("&ToPolynomial statement and FactArg statement are not allowed in the same module"); - return(1); - } - AC.topolynomialflag |= FACTARGFLAG; return(DoArgument(s,TYPEFACTARG)); } @@ -5953,9 +5948,8 @@ int CoDropSymbols(UBYTE *s) Converts the current term as much as possible to symbols. Keeps a list of all objects converted to symbols in AM.sbufnum. - Note that this cannot be executed in parallel because we have only - a single compiler buffer for this. Hence we switch on the noparallel - module option. + There is only a single compiler buffer for this. In TFORM we protect access + to it with AM.sbuflock; in ParFORM the statement is executed on the master. Option(s): OnlyFunctions [,name1][,name2][,...,namem]; @@ -5965,10 +5959,6 @@ int CoToPolynomial(UBYTE *inp) { int error = 0; while ( *inp == ' ' || *inp == ',' || *inp == '\t' ) inp++; - if ( ( AC.topolynomialflag & ~TOPOLYNOMIALFLAG ) != 0 ) { - MesPrint("&ToPolynomial statement and FactArg statement are not allowed in the same module"); - return(1); - } if ( AO.OptimizeResult.code != NULL ) { MesPrint("&Using ToPolynomial statement when there are still optimization results active."); MesPrint("&Please use #ClearOptimize instruction first."); @@ -6067,11 +6057,6 @@ int CoArgToExtraSymbol(UBYTE *s) CBUF *C = cbuf + AC.cbufnum; WORD *lhs; - /* TODO: resolve interference with rational arithmetic. (#138) */ - if ( ( AC.topolynomialflag & ~TOPOLYNOMIALFLAG ) != 0 ) { - MesPrint("&ArgToExtraSymbol statement and FactArg statement are not allowed in the same module"); - return(1); - } if ( AO.OptimizeResult.code != NULL ) { MesPrint("&Using ArgToExtraSymbol statement when there are still optimization results active."); MesPrint("&Please use #ClearOptimize instruction first."); diff --git a/sources/comtool.c b/sources/comtool.c index 06ee9263a..0fc7a5b8c 100644 --- a/sources/comtool.c +++ b/sources/comtool.c @@ -526,9 +526,11 @@ balance:; Returns -1 if the element is not in the tree. The advantage of this routine over InsTree is that this routine can be run in parallel. + Note that this is not strictly thread safe when "usage" is updated. + This update is controlled with the "updateusage" parameter. */ -int FindTree(int bufnum, WORD *subexpr) +int FindTree(int bufnum, WORD *subexpr, int updateusage) { CBUF *C = cbuf + bufnum; COMPTREE *boomlijst = C->boomlijst, *q = boomlijst + C->rootnum, *p; @@ -552,7 +554,9 @@ int FindTree(int bufnum, WORD *subexpr) else { return(-1); } } else { - p->usage++; + if ( updateusage ) { + p->usage++; + } return(p->value); } } diff --git a/sources/declare.h b/sources/declare.h index cd4fb2e40..3a1076c9b 100644 --- a/sources/declare.h +++ b/sources/declare.h @@ -1245,7 +1245,7 @@ extern int CompleteTerm(WORD *,UWORD *,UWORD *,WORD,WORD,int); extern int CodeFactors(SBYTE *s); extern WORD GenerateFactors(WORD,WORD); extern int InsTree(int,int); -extern int FindTree(int,WORD *); +extern int FindTree(int,WORD *,int); extern void RedoTree(CBUF *,int); extern void ClearTree(int); extern int CatchDollar(int); @@ -1576,8 +1576,10 @@ extern int RunHtoZArg(PHEAD WORD *fun, WORD *args); extern int NormPolyTerm(PHEAD WORD *); extern WORD ComparePoly(WORD *, WORD *, WORD); extern int ConvertToPoly(PHEAD WORD *, WORD *,WORD *,WORD); -extern int LocalConvertToPoly(PHEAD WORD *, WORD *, WORD,WORD); +extern int LocalConvertToPoly(PHEAD WORD *, WORD *, WORD,WORD,int *); extern int ConvertFromPoly(PHEAD WORD *, WORD *, WORD, WORD, WORD, WORD); +extern int LockLocalPolynomial(void); +extern void UnlockLocalPolynomial(int*); extern int FindSubterm(WORD *); extern int FindLocalSubterm(PHEAD WORD *, WORD); extern void PrintSubtermList(int,int); @@ -1606,7 +1608,7 @@ extern WORD *PolyDiv(PHEAD WORD *,WORD *,char *); extern void GCDclean(PHEAD WORD *, WORD *); extern WORD *TakeSymbolContent(PHEAD WORD *,WORD *); extern int GCDterms(PHEAD WORD *,WORD *,WORD *); -extern WORD *PutExtraSymbols(PHEAD WORD *,WORD,int *); +extern WORD *PutExtraSymbols(PHEAD WORD *,WORD,int *,int *); extern WORD *TakeExtraSymbols(PHEAD WORD *,WORD); extern WORD *MultiplyWithTerm(PHEAD WORD *, WORD *,WORD); extern WORD *TakeContent(PHEAD WORD *, WORD *); diff --git a/sources/dollar.c b/sources/dollar.c index 0b50ff52a..80e4f1859 100644 --- a/sources/dollar.c +++ b/sources/dollar.c @@ -2947,7 +2947,7 @@ int DollarFactorize(PHEAD WORD numdollar) #ifdef STEP2 WORD *tstop, pow, *r; #endif - int i, j, jj, action = 0, sign = 1; + int i, j, jj, action = 0, locked = 0, sign = 1; LONG insize, ii; WORD startebuf = cbuf[AT.ebufnum].numrhs; WORD nfactors, factorsincontent, extrafactor = 0; @@ -3144,8 +3144,9 @@ int DollarFactorize(PHEAD WORD numdollar) NewSort(BHEAD0); NewSort(BHEAD0); while ( *t ) { - if ( LocalConvertToPoly(BHEAD t,termextra,startebuf,0) < 0 ) { + if ( LocalConvertToPoly(BHEAD t,termextra,startebuf,0,&locked) < 0 ) { getout: + UnlockLocalPolynomial(&locked); AR.SortType = oldsorttype; M_free(buf1,"DollarFactorize-2"); if ( buf1content ) TermFree(buf1content,"DollarContent"); @@ -3169,6 +3170,7 @@ int DollarFactorize(PHEAD WORD numdollar) #[ Step 4: Now the hard work. */ if ( ( buf3 = poly_factorize_dollar(BHEAD buf2) ) == 0 ) { + UnlockLocalPolynomial(&locked); MesCall("DollarFactorize"); AR.SortType = oldsorttype; if ( buf2 != buf1 && buf2 ) M_free(buf2,"DollarFactorize-3"); @@ -3247,6 +3249,7 @@ int DollarFactorize(PHEAD WORD numdollar) if ( buf2 != buf1 && buf2 ) M_free(buf2,"DollarFactorize-4"); M_free(buf1,"DollarFactorize-4"); if ( buf1content ) TermFree(buf1content,"DollarContent"); + UnlockLocalPolynomial(&locked); return(0); } else { @@ -3278,6 +3281,7 @@ int DollarFactorize(PHEAD WORD numdollar) ,startebuf-numxsymbol,1) <= 0 ) { LowerSortLevel(); getout2: AR.SortType = oldsorttype; + UnlockLocalPolynomial(&locked); M_free(d->factors,"factors in dollar"); d->factors = 0; #ifdef WITHPTHREADS @@ -3336,6 +3340,7 @@ getout2: AR.SortType = oldsorttype; d->factors[i].size = t - d->factors[i].where; } } + UnlockLocalPolynomial(&locked); d->nfactors = nfactors + factorsincontent; /* #] Step 5: ConvertFromPoly diff --git a/sources/ftypes.h b/sources/ftypes.h index 999a90147..091accb73 100644 --- a/sources/ftypes.h +++ b/sources/ftypes.h @@ -1043,7 +1043,6 @@ typedef int (*TFUN1)(UBYTE *,int); #define NOLYNDON 2 #define TOPOLYNOMIALFLAG 1 -#define FACTARGFLAG 2 #define OLDFACTARG 1 #define NEWFACTARG 0 diff --git a/sources/notation.c b/sources/notation.c index 64107ad8c..635707cf7 100644 --- a/sources/notation.c +++ b/sources/notation.c @@ -290,7 +290,41 @@ WORD ComparePoly(WORD *term1, WORD *term2, WORD level) /* #] ComparePoly : - #[ ConvertToPoly : + #[ LockLocalPolynomial : +*/ +/** In TFORM, we must protect local conversions to polynomial notation, and + * back again, with a lock when the same modules can add entries to the global + * extrasymbol buffer (because we use ToPolynomial, for instance). + * Returns 1 with the lock held, or -1 when this module cannot update the + * global extrasymbol buffer and therefore needs no transaction lock. */ +int LockLocalPolynomial(void) +{ +#ifdef WITHPTHREADS + if ( AC.topolynomialflag & TOPOLYNOMIALFLAG ) { + LOCK(AM.sbuflock); + return(1); + } +#endif + return(-1); +} + +/* + #] LockLocalPolynomial : + #[ UnlockLocalPolynomial : +*/ +void UnlockLocalPolynomial(int *locked) +{ +#ifdef WITHPTHREADS + if ( *locked == 1 ) { + UNLOCK(AM.sbuflock); + } +#endif + *locked = 0; +} + +/* + #] UnlockLocalPolynomial : + #[ ConvertToPoly : */ /** * Converts a generic term to polynomial notation in which there are @@ -507,11 +541,22 @@ int ConvertToPoly(PHEAD WORD *term, WORD *outterm, WORD *comlist, WORD par) * problem when running into the already assigned ones. * This uses the FindTree for searching in the global tree and * then looks further in the AT.ebufnum. This allows fully parallel - * processing. Hence we need no locks. Cannot be used in the same - * module as ConvertToPoly. + * processing while the global tree is stable. The locked argument must + * start at zero. It changes to 1 when the first extra symbol is actually + * needed and AM.sbuflock is acquired, or to -1 when no transaction lock + * is needed for this module. The caller must hold this state until we + * have converted back, and then it passes it to UnlockLocalPolynomial. */ -int LocalConvertToPoly(PHEAD WORD *term, WORD *outterm, WORD startebuf, WORD par) +/** Helper function, which locks AM.sbuflock only when required. */ +static void EnsureLocalPolynomialLock(int *locked) +{ + if ( *locked == 0 ) { + *locked = LockLocalPolynomial(); + } +} + +int LocalConvertToPoly(PHEAD WORD *term, WORD *outterm, WORD startebuf, WORD par, int *locked) { WORD *tout, *tstop, ncoef, *t, *r, *tt, *ttwo = 0; int i, action = 0; @@ -532,6 +577,7 @@ int LocalConvertToPoly(PHEAD WORD *term, WORD *outterm, WORD startebuf, WORD par *tout++ = r[1]; } else { + EnsureLocalPolynomialLock(locked); tout[1] = SYMBOL; tout[2] = 4; tout[3] = r[0]; @@ -547,6 +593,7 @@ int LocalConvertToPoly(PHEAD WORD *term, WORD *outterm, WORD startebuf, WORD par } } else if ( *t == DOTPRODUCT ) { + EnsureLocalPolynomialLock(locked); r = t + 2; t += t[1]; while ( r < t ) { @@ -570,6 +617,7 @@ int LocalConvertToPoly(PHEAD WORD *term, WORD *outterm, WORD startebuf, WORD par } } else if ( *t == VECTOR ) { + EnsureLocalPolynomialLock(locked); r = t + 2; t += t[1]; while ( r < t ) { @@ -587,6 +635,7 @@ int LocalConvertToPoly(PHEAD WORD *term, WORD *outterm, WORD startebuf, WORD par } } else if ( *t == INDEX ) { + EnsureLocalPolynomialLock(locked); r = t + 2; t += t[1]; while ( r < t ) { @@ -615,6 +664,7 @@ int LocalConvertToPoly(PHEAD WORD *term, WORD *outterm, WORD startebuf, WORD par else { t += t[1]; } } else if ( *t >= FUNCTION ) { + EnsureLocalPolynomialLock(locked); i = FindLocalSubterm(BHEAD t,startebuf); t += t[1]; *tout++ = SYMBOL; @@ -661,65 +711,62 @@ int LocalConvertToPoly(PHEAD WORD *term, WORD *outterm, WORD startebuf, WORD par subexpressions when extra symbols have been replaced. */ +#ifdef WITHPTHREADS +/** Helper function: we only lock sbuflock if the term actually has an + * extrasymbol to replace. Search for one. */ +static int PolyHasExtraSymbols(WORD *term, WORD from, WORD to) +{ + WORD *t, *tt, *tstop, *tstop1; + + tstop = term + *term - ABS(term[*term-1]); + for ( t = term + 1; t < tstop; t += t[1] ) { + if ( *t != SYMBOL ) { + continue; + } + tstop1 = t + t[1]; + for ( tt = t + 2; tt < tstop1; tt += 2 ) { + if ( *tt >= MAXVARIABLES - to && *tt < MAXVARIABLES - from ) { + return(1); + } + } + } + return(0); +} +#endif + +static int ConvertFromPolyImpl(PHEAD WORD *term, WORD *outterm, WORD from, WORD to, WORD offset, WORD par); + int ConvertFromPoly(PHEAD WORD *term, WORD *outterm, WORD from, WORD to, WORD offset, WORD par) +{ + int locked = 0, result; + /* For LocalConvertToPoly operations, the caller already holds ebuflock if the + * the conversion introduced extrasymbols. Expression level FromPolynomial + * only needs to take the lock if the term really contains an extrasymbol. */ +#ifdef WITHPTHREADS + if ( ! par && PolyHasExtraSymbols(term,from,to) ) { + LOCK(AM.sbuflock); + locked = 1; + } +#endif + result = ConvertFromPolyImpl(BHEAD term,outterm,from,to,offset,par); +#ifdef WITHPTHREADS + if ( locked ) { + UNLOCK(AM.sbuflock); + } +#else + DUMMYUSE(locked); +#endif + return(result); +} + +static int ConvertFromPolyImpl(PHEAD WORD *term, WORD *outterm, WORD from, WORD to, WORD offset, WORD par) { WORD *tout, *tstop, *tstop1, ncoef, *t, *r, *tt; int i; -/* first = 1; */ tt = term + *term; tout = outterm+1; ncoef = ABS(tt[-1]); tstop = tt - ncoef; -/* - r = t = term + 1; - while ( t < tstop ) { - if ( *t == SYMBOL ) { - tstop1 = t + t[1]; - tt = t + 2; - while ( tt < tstop1 ) { - if ( ( *tt < MAXVARIABLES - to ) - || ( *tt >= MAXVARIABLES - from ) ) { - tt += 2; - } - else break; - } - if ( tt >= tstop1 ) { t = tstop1; continue; } - while ( r < t ) *tout++ = *r++; - t += 2; - first = 0; - while ( t < tstop1 ) { - if ( ( *t < MAXVARIABLES - to ) - || ( *t >= MAXVARIABLES - from ) ) { - *tout++ = SYMBOL; - *tout++ = 4; - *tout++ = *t++; - *tout++ = *t++; - } - else { - *tout++ = SUBEXPRESSION; - *tout++ = SUBEXPSIZE; - *tout++ = MAXVARIABLES - *t++ + offset; - *tout++ = *t++; - if ( par ) *tout++ = AT.ebufnum; - else *tout++ = AM.sbufnum; - FILLSUB(tout) - } - } - r = t; - } - else { - t += t[1]; - } - } - if ( first ) { - i = *term; t = term; - NCOPY(outterm,t,i); - return(*term); - } - while ( r < t ) *tout++ = *r++; - NCOPY(tout,tstop,ncoef) - *outterm = tout-outterm; -*/ t = term + 1; while ( t < tstop ) { if ( *t == SYMBOL ) { @@ -771,7 +818,8 @@ int ConvertFromPoly(PHEAD WORD *term, WORD *outterm, WORD from, WORD to, WORD of Searching is by tree structure. Adding changes the tree. - Notice that in TFORM we should be in sequential mode. + Writing to the global extra-symbol buffer and its search tree is protected + in TFORM by AM.sbuflock. */ int FindSubterm(WORD *subterm) @@ -841,7 +889,9 @@ int FindSubterm(WORD *subterm) Searching is by tree structure. Adding changes the tree. - Notice that in TFORM we should be in sequential mode. + Access to the global tree is protected by AM.sbuflock. In a module + that also adds global extra symbols, the caller already holds this + recursive lock for the complete local polynomial operation. */ int FindLocalSubterm(PHEAD WORD *subterm, WORD startebuf) @@ -859,7 +909,7 @@ int FindLocalSubterm(PHEAD WORD *subterm, WORD startebuf) /* First see whether we have this one already in the global buffer. */ - number = FindTree(AM.sbufnum,term); + number = FindTree(AM.sbufnum,term,0); if ( number > 0 ) goto wearehappy; /* Now look whether it is in the ebufnum between startebuf and numrhs @@ -1094,7 +1144,8 @@ void PrintExtraSymbol(int num, WORD *terms,int par) Searching is by tree structure. Adding changes the tree. - Notice that in TFORM we should be in sequential mode. + Writing to the global extra-symbol buffer and its search tree is protected + in TFORM by AM.sbuflock. */ int FindSubexpression(WORD *subexpr) diff --git a/sources/polywrap.cc b/sources/polywrap.cc index db0851075..bf068cfd6 100644 --- a/sources/polywrap.cc +++ b/sources/polywrap.cc @@ -1264,6 +1264,7 @@ int poly_factorize_expression(EXPRESSIONS expr) { // use polynomial as buffer, because it is easy to extend poly buffer(BHEAD 0); int bufpos = 0; + int locked = 0; int sumcommu = 0; // read all terms @@ -1271,12 +1272,14 @@ int poly_factorize_expression(EXPRESSIONS expr) { // substitute non-symbols by extra symbols sumcommu += DoesCommu(term); if ( sumcommu > 1 ) { + UnlockLocalPolynomial(&locked); MesPrint("ERROR: Cannot factorize an expression with more than one noncommuting object"); Terminate(-1); } buffer.check_memory(bufpos); - if (LocalConvertToPoly(BHEAD term, buffer.terms + bufpos, startebuf,0) < 0) { + if (LocalConvertToPoly(BHEAD term, buffer.terms + bufpos,startebuf,0,&locked) < 0) { /* INTERNAL_ERROR_EXCL_START */ + UnlockLocalPolynomial(&locked); MesPrint("!>ERROR: in LocalConvertToPoly [factorize_expression]"); Terminate(-1); /* INTERNAL_ERROR_EXCL_STOP */ @@ -1414,8 +1417,9 @@ int poly_factorize_expression(EXPRESSIONS expr) { for (WORD *t=buffer.terms; *t!=0; t+=*t) { // substitute extra symbols if (ConvertFromPoly(BHEAD t, term, numxsymbol, CC->numrhs-startebuf+numxsymbol, - startebuf-numxsymbol, 1) <= 0 ) { + startebuf-numxsymbol, 1) <= 0 ) { /* INTERNAL_ERROR_EXCL_START */ + UnlockLocalPolynomial(&locked); MesPrint("!>ERROR: in ConvertFromPoly [factorize_expression]"); Terminate(-1); return(-1); @@ -1430,7 +1434,10 @@ int poly_factorize_expression(EXPRESSIONS expr) { // sort and store in buffer WORD *buffer; - if (EndSort(BHEAD (WORD *)((void *)(&buffer)),2) < 0) return -1; + if (EndSort(BHEAD (WORD *)((void *)(&buffer)),2) < 0) { + UnlockLocalPolynomial(&locked); + return -1; + } LONG bufsize=0; for (WORD *t=buffer; *t!=0; t+=*t) @@ -1495,6 +1502,7 @@ int poly_factorize_expression(EXPRESSIONS expr) { // final sorting if (EndSort(BHEAD NULL,0) < 0) { LowerSortLevel(); + UnlockLocalPolynomial(&locked); Terminate(-1); } @@ -1511,6 +1519,7 @@ int poly_factorize_expression(EXPRESSIONS expr) { strcpy((char*)AC.Commercial, oldCommercial); poly_free_poly_vars(BHEAD "AN.poly_vars_factorize_expression"); + UnlockLocalPolynomial(&locked); return 0; } diff --git a/sources/ratio.c b/sources/ratio.c index e1f29daa4..63f62043c 100644 --- a/sources/ratio.c +++ b/sources/ratio.c @@ -1149,7 +1149,7 @@ WORD *GCDfunction3(PHEAD WORD *in1, WORD *in2) GETBIDENTITY WORD oldsorttype = AR.SortType, *ow = AT.WorkPointer;; WORD *t, *tt, *gcdout, *term1, *term2, *confree1, *confree2, *gcdout1, *proper1, *proper2; - int i, actionflag1, actionflag2; + int i, actionflag1, actionflag2, locked = 0; WORD startebuf = cbuf[AT.ebufnum].numrhs; WORD tryterm1, tryterm2; if ( in2[*in2] == 0 ) { t = in1; in1 = in2; in2 = t; } @@ -1187,7 +1187,7 @@ WORD *GCDfunction3(PHEAD WORD *in1, WORD *in2) Now we have to replace all non-symbols and symbols to a negative power by extra symbols. */ - if ( ( proper1 = PutExtraSymbols(BHEAD confree1,startebuf,&actionflag1) ) == 0 ) goto CalledFrom; + if ( ( proper1 = PutExtraSymbols(BHEAD confree1,startebuf,&actionflag1,&locked) ) == 0 ) goto CalledFrom; if ( confree1 != in1 ) { if ( tryterm1 ) { TermFree(confree1,"TakeContent"); } else { M_free(confree1,"TakeContent"); } @@ -1195,7 +1195,7 @@ WORD *GCDfunction3(PHEAD WORD *in1, WORD *in2) /* TermFree(confree1,"TakeSymbolContent"); */ - if ( ( proper2 = PutExtraSymbols(BHEAD confree2,startebuf,&actionflag2) ) == 0 ) goto CalledFrom; + if ( ( proper2 = PutExtraSymbols(BHEAD confree2,startebuf,&actionflag2,&locked) ) == 0 ) goto CalledFrom; if ( confree2 != in2 ) { if ( tryterm2 ) { TermFree(confree2,"TakeContent"); } else { M_free(confree2,"TakeContent"); } @@ -1220,6 +1220,7 @@ WORD *GCDfunction3(PHEAD WORD *in1, WORD *in2) } cbuf[AT.ebufnum].numrhs = startebuf; + UnlockLocalPolynomial(&locked); /* Now multiply gcdout by term1 */ @@ -1234,6 +1235,7 @@ WORD *GCDfunction3(PHEAD WORD *in1, WORD *in2) AT.WorkPointer = ow; return(gcdout); CalledFrom: + UnlockLocalPolynomial(&locked); AN.tryterm = 0; MLOCK(ErrorMessageLock); MesCall("GCDfunction3"); @@ -1246,14 +1248,14 @@ WORD *GCDfunction3(PHEAD WORD *in1, WORD *in2) #[ PutExtraSymbols : */ -WORD *PutExtraSymbols(PHEAD WORD *in,WORD startebuf,int *actionflag) +WORD *PutExtraSymbols(PHEAD WORD *in,WORD startebuf,int *actionflag,int *locked) { WORD *termout = AT.WorkPointer; int action; *actionflag = 0; NewSort(BHEAD0); while ( *in ) { - if ( ( action = LocalConvertToPoly(BHEAD in,termout,startebuf,0) ) < 0 ) { + if ( ( action = LocalConvertToPoly(BHEAD in,termout,startebuf,0,locked) ) < 0 ) { LowerSortLevel(); goto CalledFrom; } @@ -2850,7 +2852,7 @@ int DIVfunction(PHEAD WORD *term,WORD level,int par) WORD *t, *tt, *r, *arg1 = 0, *arg2 = 0, *arg3 = 0, *termout; WORD *tstop, *tend, *r3, *rr, *rstop, tlength, rlength, newlength; WORD *proper1, *proper2, *proper3 = 0, numdol = -1; - int numargs = 0, type1, type2, actionflag1, actionflag2; + int numargs = 0, type1, type2, actionflag1, actionflag2, locked = 0; WORD startebuf = cbuf[AT.ebufnum].numrhs; int division = ( par <= 2 ); /* false for mul_ */ if ( par < 0 || par > 3 ) { @@ -2939,8 +2941,8 @@ divzero:; M_free(arg1,"DIVfunction"); return(0); } - if ( ( proper1 = PutExtraSymbols(BHEAD arg1,startebuf,&actionflag1) ) == 0 ) goto CalledFrom; - if ( ( proper2 = PutExtraSymbols(BHEAD arg2,startebuf,&actionflag2) ) == 0 ) goto CalledFrom; + if ( ( proper1 = PutExtraSymbols(BHEAD arg1,startebuf,&actionflag1,&locked) ) == 0 ) goto CalledFrom; + if ( ( proper2 = PutExtraSymbols(BHEAD arg2,startebuf,&actionflag2,&locked) ) == 0 ) goto CalledFrom; /* if ( type2 == 0 ) M_free(arg2,"DIVfunction"); else { @@ -2974,6 +2976,7 @@ divzero:; M_free(proper2,"DIVfunction"); M_free(proper1,"DIVfunction"); cbuf[AT.ebufnum].numrhs = startebuf; + UnlockLocalPolynomial(&locked); if ( *arg3 ) { termout = AT.WorkPointer; tlength = tend[-1]; @@ -3004,6 +3007,7 @@ divzero:; M_free(arg3,"DIVfunction"); return(0); CalledFrom: + UnlockLocalPolynomial(&locked); MLOCK(ErrorMessageLock); MesCall("DIVfunction"); MUNLOCK(ErrorMessageLock); diff --git a/sources/structs.h b/sources/structs.h index 707e3e0f1..4b2eff842 100644 --- a/sources/structs.h +++ b/sources/structs.h @@ -1438,7 +1438,7 @@ struct M_const { #ifdef WITHPTHREADS pthread_rwlock_t handlelock; /* (M) */ pthread_mutex_t storefilelock; /* (M) */ - pthread_mutex_t sbuflock; /* (M) Lock for writing in the AM.sbuffer */ + pthread_mutex_t sbuflock; /* (M) Recursive lock for the AM.sbuffer and local polynomial conversions */ LONG ThreadScratSize; /* (M) Size of Fscr[0/2] buffers of the workers */ LONG ThreadScratOutSize; /* (M) Size of Fscr[1] buffers of the workers */ #endif @@ -1843,7 +1843,7 @@ struct C_const { int NoCompress; /* (R) Controls native compression */ int IsFortran90; /* Tells whether the Fortran is Fortran90 */ int MultiBracketLevels; /* Number of elements in MultiBracketBuf */ - int topolynomialflag; /* To avoid ToPolynomial and FactArg together */ + int topolynomialflag; /* Module can add entries to the global extra-symbol buffer */ int ffbufnum; /* Buffer number for user defined factorizations */ int OldFactArgFlag; int MemDebugFlag; /* Only used when MALLOCDEBUG in tools.c */ diff --git a/sources/threads.c b/sources/threads.c index 9dc5760c0..af5cae917 100644 --- a/sources/threads.c +++ b/sources/threads.c @@ -321,7 +321,9 @@ int StartAllThreads(int number) AR.infile = &(AR.Fscr[0]); AR.outfile = &(AR.Fscr[1]); AR.hidefile = &(AR.Fscr[2]); - AM.sbuflock = dummylock; + /* This lock must be recursive; it is held during local polynomial conversions, + * and FindSubterm and FindSubexpression take the lock also. */ + INIRECLOCK(AM.sbuflock); AS.inputslock = dummylock; AS.outputslock = dummylock; AS.MaxExprSizeLock = dummylock; @@ -4773,4 +4775,4 @@ extern void optimize_expression_given_Horner_threaded(void) { #] optimize_expression_given_Horner_threaded : */ -#endif \ No newline at end of file +#endif