From 32396dc6770caeefbac6fef7d760befd3c3fc74c Mon Sep 17 00:00:00 2001 From: Wouter Wijngaards Date: Fri, 25 Jan 2008 15:45:54 +0000 Subject: [PATCH] more fixes, more tests. git-svn-id: file:///svn/unbound/trunk@903 be551aaa-1e26-0410-a405-d3ace91eadb9 --- doc/Changelog | 4 ++++ libunbound/context.c | 21 +++++++++++++++------ libunbound/context.h | 7 +++++-- libunbound/worker.c | 26 ++++++++++++++++---------- libunbound/worker.h | 2 ++ testdata/05-asynclook.tpkg | Bin 1675 -> 1730 bytes 6 files changed, 42 insertions(+), 18 deletions(-) diff --git a/doc/Changelog b/doc/Changelog index dfa6297cf..f4fcc428e 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -11,6 +11,10 @@ - fix pass async_id=NULL to _async resolve(). - rewrote _wait() routine, so that it is threadsafe. - cancelation is threadsafe. + - asynclook extended test in tpkg. + - fixed two races where forked bg process waits for (somehow shared?) + locks, so does not service the query pipe on the bg side. + Now those locks are only held for fg_threads and for bg_as_a_thread. 24 January 2008: Wouter - tested the cancel() function. diff --git a/libunbound/context.c b/libunbound/context.c index c1436121f..26a03c8ed 100644 --- a/libunbound/context.c +++ b/libunbound/context.c @@ -162,16 +162,20 @@ context_new(struct ub_val_ctx* ctx, char* name, int rrtype, int rrclass, } struct alloc_cache* -context_obtain_alloc(struct ub_val_ctx* ctx) +context_obtain_alloc(struct ub_val_ctx* ctx, int locking) { struct alloc_cache* a; int tnum = 0; - lock_basic_lock(&ctx->cfglock); + if(locking) { + lock_basic_lock(&ctx->cfglock); + } a = ctx->alloc_list; if(a) ctx->alloc_list = a->super; /* snip off list */ else tnum = ctx->thr_next_num++; - lock_basic_unlock(&ctx->cfglock); + if(locking) { + lock_basic_unlock(&ctx->cfglock); + } if(a) { a->super = &ctx->superalloc; return a; @@ -184,14 +188,19 @@ context_obtain_alloc(struct ub_val_ctx* ctx) } void -context_release_alloc(struct ub_val_ctx* ctx, struct alloc_cache* alloc) +context_release_alloc(struct ub_val_ctx* ctx, struct alloc_cache* alloc, + int locking) { if(!ctx || !alloc) return; - lock_basic_lock(&ctx->cfglock); + if(locking) { + lock_basic_lock(&ctx->cfglock); + } alloc->super = ctx->alloc_list; ctx->alloc_list = alloc; - lock_basic_unlock(&ctx->cfglock); + if(locking) { + lock_basic_unlock(&ctx->cfglock); + } } uint8_t* diff --git a/libunbound/context.h b/libunbound/context.h index 5be7dc8ac..80b10d967 100644 --- a/libunbound/context.h +++ b/libunbound/context.h @@ -231,16 +231,19 @@ struct ctx_query* context_new(struct ub_val_ctx* ctx, char* name, int rrtype, /** * Get a new alloc. Creates a new one or uses a cached one. * @param ctx: context + * @param locking: if true, cfglock is locked while getting alloc. * @return an alloc, or NULL on mem error. */ -struct alloc_cache* context_obtain_alloc(struct ub_val_ctx* ctx); +struct alloc_cache* context_obtain_alloc(struct ub_val_ctx* ctx, int locking); /** * Release an alloc. Puts it into the cache. * @param ctx: context + * @param locking: if true, cfglock is locked while releasing alloc. * @param alloc: alloc to relinquish. */ -void context_release_alloc(struct ub_val_ctx* ctx, struct alloc_cache* alloc); +void context_release_alloc(struct ub_val_ctx* ctx, struct alloc_cache* alloc, + int locking); /** * Serialize a context query that questions data. diff --git a/libunbound/worker.c b/libunbound/worker.c index c387b7f74..feb1985b7 100644 --- a/libunbound/worker.c +++ b/libunbound/worker.c @@ -74,7 +74,8 @@ libworker_delete(struct libworker* w) if(!w) return; if(w->env) { mesh_delete(w->env->mesh); - context_release_alloc(w->ctx, w->env->alloc); + context_release_alloc(w->ctx, w->env->alloc, + !w->is_bg || w->is_bg_thread); ldns_buffer_free(w->env->scratch_buffer); regional_destroy(w->env->scratch); ub_randfree(w->env->rnd); @@ -90,12 +91,13 @@ libworker_delete(struct libworker* w) /** setup fresh libworker struct */ static struct libworker* -libworker_setup(struct ub_val_ctx* ctx) +libworker_setup(struct ub_val_ctx* ctx, int is_bg) { unsigned int seed; struct libworker* w = (struct libworker*)calloc(1, sizeof(*w)); struct config_file* cfg = ctx->env->cfg; if(!w) return NULL; + w->is_bg = is_bg; w->ctx = ctx; w->env = (struct module_env*)malloc(sizeof(*w->env)); if(!w->env) { @@ -103,7 +105,7 @@ libworker_setup(struct ub_val_ctx* ctx) return NULL; } *w->env = *ctx->env; - w->env->alloc = context_obtain_alloc(ctx); + w->env->alloc = context_obtain_alloc(ctx, !w->is_bg || w->is_bg_thread); if(!w->env->alloc) { libworker_delete(w); return NULL; @@ -125,16 +127,20 @@ libworker_setup(struct ub_val_ctx* ctx) seed = (unsigned int)time(NULL) ^ (unsigned int)getpid() ^ (((unsigned int)w->thread_num)<<17); seed ^= (unsigned int)w->env->alloc->next_id; - lock_basic_lock(&ctx->cfglock); - /* Openssl RAND_... functions are not as threadsafe as documented, - * put a lock around them. */ + if(!w->is_bg || w->is_bg_thread) { + /* Openssl RAND_... functions are not as threadsafe + * as documented, put a lock around them. */ + lock_basic_lock(&ctx->cfglock); + } if(!ub_initstate(seed, w->env->rnd, RND_STATE_SIZE)) { lock_basic_unlock(&ctx->cfglock); seed = 0; libworker_delete(w); return NULL; } - lock_basic_unlock(&ctx->cfglock); + if(!w->is_bg || w->is_bg_thread) { + lock_basic_unlock(&ctx->cfglock); + } seed = 0; w->base = comm_base_create(); @@ -311,7 +317,7 @@ int libworker_bg(struct ub_val_ctx* ctx) lock_basic_lock(&ctx->cfglock); if(ctx->dothread) { lock_basic_unlock(&ctx->cfglock); - w = libworker_setup(ctx); + w = libworker_setup(ctx, 1); w->is_bg_thread = 1; #ifdef ENABLE_LOCK_CHECKS w->thread_num = 1; /* for nicer DEBUG checklocks */ @@ -322,7 +328,7 @@ int libworker_bg(struct ub_val_ctx* ctx) lock_basic_unlock(&ctx->cfglock); switch((ctx->bg_pid=fork())) { case 0: - w = libworker_setup(ctx); + w = libworker_setup(ctx, 1); if(!w) fatal_exit("out of memory"); /* close non-used parts of the pipes */ close(ctx->qqpipe[1]); @@ -490,7 +496,7 @@ setup_qinfo_edns(struct libworker* w, struct ctx_query* q, int libworker_fg(struct ub_val_ctx* ctx, struct ctx_query* q) { - struct libworker* w = libworker_setup(ctx); + struct libworker* w = libworker_setup(ctx, 0); uint16_t qflags, qid; struct query_info qinfo; struct edns_data edns; diff --git a/libunbound/worker.h b/libunbound/worker.h index f1969b99d..275f10463 100644 --- a/libunbound/worker.h +++ b/libunbound/worker.h @@ -68,6 +68,8 @@ struct libworker { /** context we are operating under */ struct ub_val_ctx* ctx; + /** is this the bg worker? */ + int is_bg; /** is this a bg worker that is threaded (not forked)? */ int is_bg_thread; diff --git a/testdata/05-asynclook.tpkg b/testdata/05-asynclook.tpkg index 4e4b5b6d888627e3430202390a45f0c6331c0678..e1d7d555c47a838a2e218aa72abb73b072ea0b77 100644 GIT binary patch literal 1730 zc-jHN20i&7iwFQ9{h3Dq1MOP-bJ|7__OIMuv5_@?Op1_%ffffE0=RJ!V+WJEnNEiY z=>XM2QuGLB+Wzm|lXwVhz|#Po#P@k@&Ubsed;8sM&z5s(%^%siS92`s(J8nxZIX(+W%|y5X5v|w>6msVsi$j_xEG{dn7TDjV=M~g z?$D4drw3~JK!JmJyyvx&_$)a*RxvI-I5=Gk9vJXYGh?!R>U}}70|$BzgzgU!S?~hQ zve{y516qa`e&8X0OlYVDdcRD%p{)m|V@G=0v5t1=9v#D9By~~wt^MJu-D-X;DXL6T zHA& zAc(5KD#{sye#zLOWlfk!rQ#47j!gmmWL8A{A+V;7N448iEFl>)-K-hX^mEdcMRnJ4 zv~Y%_Rl_vEc7ix?&mzq%*rF22-(i52B}RG_#935kMtqCnsFZOh<~#Xkj;-koWnAbu z&Cqf*eItGj-mm^HfA4G@kHMPr-|>SD*jRo3 z%Q^LAdj7LAp8xrLj-UU#z(eQ1!}iD>-T)F-CP)4%8;?Hv*$4~=M%ajoamRCpHbjcB z;nuV)wt=AsQE_toC60mU$VAbBc1ytzvEOP*5^dM=H?{MvUjOkO;;U_}S^tGXV*M*@ z{TEa<&)5Gha0={pHo3n=mv3BoM7(UC|8m{{dZ!Isi%NJ)kcil7y@PCCmI*nHE)uS1 z4%ngpIi%i*)Eju$X}_vtKXOrNRbPYS>W)F-b1-u8Co@Ev^{aOu;e8V-6=+=6o6XB6 zsnp(lxNhN1#2vLwgIw3Et@FzUy#G+YW(D#xb>2uduBQ!^#`R6TIjK%5YC)3mOBs7m zZ`8>4oEZ*>%x-1Tn@N32ktJ1@va)m{@5&){nMdlPOKQ?3#{c^D|Mhch&H69ovxUU_ zkIc^hf~@fMzY9EY{>M4eck}{q)vT8UW?B;1$sfsMDit06VEQcki4(IjW@9~gIo(j^ zvuj*WQer=ZzflvWg<-(rA&~#vmAwkYiD+W$5GJjYye{@I;j=)f)6i{nIWUW#Rv zOl+J*vPT1NFg1#S&@qDzC`_5hed%qz#BMvxrTB587hQf9ZaT3~5Q#?ucpRA17#`^c zh!}HByG154GjJ@B6^+T)k-JDJGi{uYg09(YaGlV!jG2PCv+FrMPaDh)dqR373QRV1 zeK^AHj_LtL6#>LIo$-^;VR^xP^jtIL_NB0#+31yJj^0DEIbE}gR)?TRVW zz<4r&C6ncA!@|y@t;OjP%6A&Lq%C!sF?EM51zRR+v-+O~SK}=Sw=$htmd4&em-44? znZM2IU%}sawA9!>dz&`($aS`Dsy1%o`)6ys(mIx--?D6VfLYg43OifoZ?pQJ##Xsv zxow-SjhlEHjE$EZy$h&q;02rg=1bEIVKK^gcqi92tMB-MaF2pS<1|AMe})ps%luvH z;!+2KH9iE}af1bR{5MixAc-);&YqH2zyBD%z6l#^-hWT>`RV&_HeZPU|0B!ae|CXa znok?rfEMT8fXJ`Z^YKQC`-CKu8ufM0bc3j(wU|gl$R1_DGCQH&aYEZ5Rraoc!Zj^G zy=R^YZ!{a!9LPTvb8<18gKE76sx1FVE;Q2)@SA!?Fgyuvq^eZG5A#pEr)i$ylPW6d zIo_xUr0NV@$EJ2rB-MdYqb_bPKGx4luc_r0$(6S-9QH3QN&o-= literal 1675 zc-jGr26Xu!iwFR~zL`e=1MOP-bJ|7__OIMuv5_@?Op1_%ffffQ1aRXd#ttTRGo20* z(t%YANzo(N)AoPwo+KW^20RUrNqnEj=6tued$-@cc5S(k)%-hKw;bn2GE6VGvwp~k zQmGJs7LQV&33e2@bX3geODdC7@`a=P4iq*pmU{^OK=S~0Mot(|Z?W8}_U{z(JpZW* z*Q4vOfhf-LKEuDN9UDY=&3ZrXkgBk4mtkt9B56JdGl5$eu-V7uZlad~P!rtCk(F5&hCG@dn zQXNn~fWG5_X+u=TN>l5hNmuiI(;g(I(j8J(QcZ-kTaE6;dAm~)4-PP>K(%_$4*CmMQZ)FH<{3CQ6mPaGmSOu@CSv=< z?8lRO#^xY=8eNTkhH!(p(-@0F zwLdZ>%Nc-L-BVyMp6_X+B0ig)9;-+S4^B?kf(Ir%(#)7FpL$=A?7)G61EKp9b2 zvuw54+K^V^g&%mx9}^mBfqq>j{m|9})3GBx-B?FAbPo>UZ<0By{oeg>(e1Q9Ruokx znXB4qr`vhmZeCXM_$#xDd$H}aS_(Nb#J&$TP$5=u#rn*@(Wa;PGq0Fnh zjJ<^`9IYCr0k#vwj(ZkqX3iFsNd6WbtSm4xpde1YGBx5`6lbL*o!HpPzbUpRE0l4e z<1$0b(ejP>*?(u^&a_z(r|ddju+N5yTuLNO6pWXaaIcF%$hSV(I39x)_rK!@Yp}8W z{+A2t(d7PTWxW53#R9+ow}FT5e}|osTYLe`SQ%gW%WORQ>Sr@B9Js?qRE#H{GqNF4 zgbg>QWw8ScM-Uaq*Wb)F5M7xlI@E3`_#yUNE=i)}TK=w1zUBKrenWhzPA#>3yHL)K#t99zH!EtrRpzt}kbMYrLMB2@ZcOT(>8)`LZoj2R< z^ERn9-h8<1;7i24>zW3+Y}Pxc=Ph{up?S#)obZhsj`%pr6YM;15y`_NSz;&Iy)rB|LXn!^=oX!{x7Mi z{7-WK^ZmaSJaGTVG13o|0k~*4D*`hu3GC*NPDU3B$rLVEz!u|L)4N0&yTZb94yf)^S`Hk8ozp0-=^qNfa$6fmm9KMU_k} zi6U9jz#C4Cq9b&SU;_#h=J8y5+pMtH4jWROoESuppShP#JSK?5BLTb)%!v;VbOS{6 zIfmUL6PX$~6v&ds+3d(|B$S&proEtRHk({8G%aJQAnxpX&cM@#scDZ%k3@mVhOQ3> zc-&Dvpr|5%_@$GieDN-ZY`0C* zwRRIvh%pQWSkL5-fWAOx^uYI+EBz6%JpW_(`Z{c^$p0P{inp%kjwc3)SDepW%*}vrkQqt-_$FE;Yn~M)ukGKn19&=P4f(&)KSU6 z@$Sk%>dw%0Y-$H(QXd)(>QdYAv3^>5O)a-fF1)$vuzzVG7mgpyS7Q}n6Dr@&fRS2w zd&15wRC}TMJ~hhZL^G`rmGe`JgM)*EgM)*EgM)*EgM)*EgM)*EgM)*EgM)*EgM)*E VgM)*EgM;H6;y*PZ8t(v5007~3M_vE` -- 2.47.2