From f00e10dfcb19faeeabb42c5532f4b47f5548be97 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Mon, 17 Jan 2005 01:21:43 +0000 Subject: [PATCH] Fix bug #249: rstemp sets fail. Problem was that rset files were closed when they shouldn't. Added function rfd_is_last that returns 1 if the RSFD supplied is the last one; 0 otherwise. Added a use_list member in the RSET which is a list of RSFDs in an RSET (opposite of free_list). This means we can track RSFD leaks. --- include/rset.h | 4 +- rset/rset.c | 157 +++++++++++++++++++++++++++++++------------------------- rset/rstemp.c | 148 ++++++++++++++++++++++++++-------------------------- 3 files changed, 166 insertions(+), 143 deletions(-) diff --git a/include/rset.h b/include/rset.h index 0352a1c..87b47ae 100644 --- a/include/rset.h +++ b/include/rset.h @@ -1,4 +1,4 @@ -/* $Id: rset.h,v 1.46 2005-01-16 23:14:56 adam Exp $ +/* $Id: rset.h,v 1.47 2005-01-17 01:21:43 adam Exp $ Copyright (C) 1995-2005 Index Data ApS @@ -142,6 +142,7 @@ typedef struct rset char my_nmem; /* Should the nmem be destroyed with the rset? */ /* 1 if created with it, 0 if passed from above */ RSFD free_list; /* all rfd's allocated but not currently in use */ + RSFD use_list; /* all rfd's in use */ int scope; /* On what level do we count hits and compare them? */ TERMID term; /* the term thing for ranking etc */ } rset; @@ -164,6 +165,7 @@ typedef struct rset RSFD rfd_create_base(RSET rs); void rfd_delete_base(RSFD rfd); +int rfd_is_last(RSFD rfd); RSET rset_create_base(const struct rset_control *sel, NMEM nmem, diff --git a/rset/rset.c b/rset/rset.c index f5bcdb5..41d39a7 100644 --- a/rset/rset.c +++ b/rset/rset.c @@ -1,4 +1,4 @@ -/* $Id: rset.c,v 1.42 2005-01-15 19:38:34 adam Exp $ +/* $Id: rset.c,v 1.43 2005-01-17 01:21:44 adam Exp $ Copyright (C) 1995-2005 Index Data ApS @@ -20,8 +20,6 @@ Free Software Foundation, 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. */ - - #include #include #include @@ -29,56 +27,62 @@ Free Software Foundation, 59 Temple Place - Suite 330, Boston, MA #include #include -static int log_level=0; -static int log_level_initialized=0; +static int log_level = 0; +static int log_level_initialized = 0; -/** +/** \fn rfd_create_base(RSET rs) + * * creates an rfd. Either allocates a new one, in which case the priv * pointer is null, and will have to be filled in, or picks up one * from the freelist, in which case the priv is already allocated, * and presumably everything that hangs from it as well */ - RSFD rfd_create_base(RSET rs) { - RSFD rnew=rs->free_list; - - if (!log_level_initialized) - { - log_level=yaz_log_module_level("rset"); - log_level_initialized=1; - } + RSFD rnew = rs->free_list; if (rnew) { - rs->free_list=rnew->next; + rs->free_list = rnew->next; assert(rnew->rset==rs); - yaz_log(log_level,"rfd-create_base (fl): rfd=%p rs=%p fl=%p priv=%p", - rnew, rs, rs->free_list, rnew->priv); - } else { - rnew=nmem_malloc(rs->nmem, sizeof(*rnew)); - rnew->priv=NULL; - rnew->rset=rs; - yaz_log(log_level,"rfd_create_base (new): rfd=%p rs=%p fl=%p priv=%p", - rnew, rs, rs->free_list, rnew->priv); + yaz_log(log_level, "rfd_create_base (fl): rfd=%p rs=%p fl=%p priv=%p", + rnew, rs, rs->free_list, rnew->priv); + } + else + { + rnew = nmem_malloc(rs->nmem, sizeof(*rnew)); + rnew->priv = NULL; + rnew->rset = rs; + yaz_log(log_level, "rfd_create_base (new): rfd=%p rs=%p fl=%p priv=%p", + rnew, rs, rs->free_list, rnew->priv); } - rnew->next=NULL; /* not part of any (free?) list */ + rnew->next = rs->use_list; + rs->use_list = rnew; return rnew; } -/* puts an rfd into the freelist of the rset. Only when the rset gets */ -/* deleted, will all the nmem disappear */ +/** \fn rfd_delete_base + * + * puts an rfd into the freelist of the rset. Only when the rset gets + * deleted, will all the nmem disappear */ void rfd_delete_base(RSFD rfd) { - RSET rs=rfd->rset; - yaz_log(log_level,"rfd_delete_base: rfd=%p rs=%p priv=%p fl=%p", + RSFD *pfd; + RSET rs = rfd->rset; + yaz_log(log_level, "rfd_delete_base: rfd=%p rs=%p priv=%p fl=%p", rfd, rs, rfd->priv, rs->free_list); - assert(NULL == rfd->next); - rfd->next=rs->free_list; - rs->free_list=rfd; + for (pfd = &rs->use_list; *pfd; pfd = &(*pfd)->next) + if (*pfd == rfd) + { + *pfd = (*pfd)->next; + rfd->next = rs->free_list; + rs->free_list = rfd; + return; + } + yaz_log(YLOG_WARN, "rset_close handle not found. type=%s", + rs->control->desc); } - RSET rset_create_base(const struct rset_control *sel, NMEM nmem, const struct key_control *kcontrol, int scope, TERMID term) @@ -86,73 +90,90 @@ RSET rset_create_base(const struct rset_control *sel, RSET rnew; NMEM M; /* assert(nmem); */ /* can not yet be used, api/t4 fails */ + if (!log_level_initialized) + { + log_level = yaz_log_module_level("rset"); + log_level_initialized = 1; + } + if (nmem) - M=nmem; + M = nmem; else - M=nmem_create(); - rnew = (RSET) nmem_malloc(M,sizeof(*rnew)); - yaz_log (log_level, "rs_create(%s) rs=%p (nm=%p)", sel->desc, rnew, nmem); - rnew->nmem=M; + M = nmem_create(); + rnew = (RSET) nmem_malloc(M, sizeof(*rnew)); + yaz_log(log_level, "rs_create(%s) rs=%p (nm=%p)", sel->desc, rnew, nmem); + rnew->nmem = M; if (nmem) - rnew->my_nmem=0; + rnew->my_nmem = 0; else - rnew->my_nmem=1; + rnew->my_nmem = 1; rnew->control = sel; rnew->count = 1; /* refcount! */ rnew->priv = 0; - rnew->free_list=NULL; - rnew->keycontrol=kcontrol; - rnew->scope=scope; - rnew->term=term; + rnew->free_list = NULL; + rnew->use_list = NULL; + rnew->keycontrol = kcontrol; + rnew->scope = scope; + rnew->term = term; if (term) - term->rset=rnew; + term->rset = rnew; return rnew; } void rset_delete (RSET rs) { (rs->count)--; - yaz_log(log_level,"rs_delete(%s), rs=%p, count=%d", + yaz_log(log_level, "rs_delete(%s), rs=%p, count=%d", rs->control->desc, rs, rs->count); if (!rs->count) { + if (rs->use_list) + yaz_log(YLOG_WARN, "rs_delete(%s) still has RFDs in use", + rs->control->desc); (*rs->control->f_delete)(rs); if (rs->my_nmem) nmem_destroy(rs->nmem); } } +int rfd_is_last(RSFD rfd) +{ + if (rfd->rset->use_list == rfd && rfd->next == 0) + return 1; + return 0; +} + RSET rset_dup (RSET rs) { (rs->count)++; - yaz_log(log_level,"rs_dup(%s), rs=%p, count=%d", + yaz_log(log_level, "rs_dup(%s), rs=%p, count=%d", rs->control->desc, rs, rs->count); return rs; } int rset_default_forward(RSFD rfd, void *buf, TERMID *term, - const void *untilbuf) + const void *untilbuf) { - int more=1; - int cmp=rfd->rset->scope; + int more = 1; + int cmp = rfd->rset->scope; if (log_level) { yaz_log (log_level, "rset_default_forward starting '%s' (ct=%p rfd=%p)", rfd->rset->control->desc, rfd->rset, rfd); /* key_logdump(log_level, untilbuf); */ } - while ( (cmp>=rfd->rset->scope) && (more)) + while (cmp>=rfd->rset->scope && more) { if (log_level) /* time-critical, check first */ - yaz_log(log_level,"rset_default_forward looping m=%d c=%d",more,cmp); - more=rset_read(rfd, buf, term); + yaz_log(log_level, "rset_default_forward looping m=%d c=%d", + more, cmp); + more = rset_read(rfd, buf, term); if (more) - cmp=(rfd->rset->keycontrol->cmp)(untilbuf,buf); -/* if (more) - key_logdump(log_level,buf); */ + cmp = (rfd->rset->keycontrol->cmp)(untilbuf, buf); } if (log_level) - yaz_log (log_level, "rset_default_forward exiting m=%d c=%d",more,cmp); + yaz_log (log_level, "rset_default_forward exiting m=%d c=%d", + more, cmp); return more; } @@ -165,11 +186,11 @@ int rset_default_forward(RSFD rfd, void *buf, TERMID *term, */ zint rset_count(RSET rs) { - double cur,tot; - RSFD rfd=rset_open(rs,0); - rset_pos(rfd,&cur,&tot); + double cur, tot; + RSFD rfd = rset_open(rs, 0); + rset_pos(rfd, &cur, &tot); rset_close(rfd); - return (zint)(tot); + return (zint) tot; } @@ -185,7 +206,7 @@ void rset_get_one_term(RSET ct,TERMID *terms,int maxterms,int *curterm) if (ct->term) { if (*curterm < maxterms) - terms[*curterm]=ct->term; + terms[*curterm] = ct->term; (*curterm)++; } } @@ -198,25 +219,23 @@ TERMID rset_term_create (const char *name, int length, const char *flags, TERMID t; yaz_log (log_level, "term_create '%s' %d f=%s type=%d nmem=%p", name, length, flags, type, nmem); - t= (TERMID) nmem_malloc (nmem, sizeof(*t)); + t= (TERMID) nmem_malloc(nmem, sizeof(*t)); if (!name) t->name = NULL; else if (length == -1) - t->name = nmem_strdup(nmem,name); + t->name = nmem_strdup(nmem, name); else { - t->name = (char*) nmem_malloc(nmem,length+1); + t->name = (char*) nmem_malloc(nmem, length+1); memcpy (t->name, name, length); t->name[length] = '\0'; } if (!flags) t->flags = NULL; else - t->flags = nmem_strdup(nmem,flags); + t->flags = nmem_strdup(nmem, flags); t->type = type; - t->rankpriv=0; - t->rset=0; + t->rankpriv = 0; + t->rset = 0; return t; } - - diff --git a/rset/rstemp.c b/rset/rstemp.c index 99c4543..57e1beb 100644 --- a/rset/rstemp.c +++ b/rset/rstemp.c @@ -1,4 +1,4 @@ -/* $Id: rstemp.c,v 1.59 2005-01-17 00:01:51 adam Exp $ +/* $Id: rstemp.c,v 1.60 2005-01-17 01:21:44 adam Exp $ Copyright (C) 1995-2005 Index Data ApS @@ -35,14 +35,14 @@ Free Software Foundation, 59 Temple Place - Suite 330, Boston, MA #include #include -static RSFD r_open (RSET ct, int flag); -static void r_close (RSFD rfd); -static void r_delete (RSET ct); -static int r_read (RSFD rfd, void *buf, TERMID *term); -static int r_write (RSFD rfd, const void *buf); -static void r_pos (RSFD rfd, double *current, double *total); -static void r_flush (RSFD rfd, int mk); -static void r_reread (RSFD rfd); +static RSFD r_open(RSET ct, int flag); +static void r_close(RSFD rfd); +static void r_delete(RSET ct); +static int r_read(RSFD rfd, void *buf, TERMID *term); +static int r_write(RSFD rfd, const void *buf); +static void r_pos(RSFD rfd, double *current, double *total); +static void r_flush(RSFD rfd, int mk); +static void r_reread(RSFD rfd); static const struct rset_control control = @@ -80,25 +80,24 @@ struct rset_temp_rfd { zint cur; /* number of the current hit */ }; -static int log_level=0; -static int log_level_initialized=0; +static int log_level = 0; +static int log_level_initialized = 0; RSET rstemp_create( NMEM nmem, const struct key_control *kcontrol, - int scope, - const char *temp_path, TERMID term) + int scope, const char *temp_path, TERMID term) { - RSET rnew=rset_create_base(&control, nmem, kcontrol, scope,term); + RSET rnew = rset_create_base(&control, nmem, kcontrol, scope,term); struct rset_temp_info *info; if (!log_level_initialized) { - log_level=yaz_log_module_level("rstemp"); - log_level_initialized=1; + log_level = yaz_log_module_level("rstemp"); + log_level_initialized = 1; } info = (struct rset_temp_info *) nmem_malloc(rnew->nmem, sizeof(*info)); info->fd = -1; info->fname = NULL; info->buf_size = 4096; - info->buf_mem = (char *) nmem_malloc (rnew->nmem, info->buf_size); + info->buf_mem = (char *) nmem_malloc(rnew->nmem, info->buf_size); info->pos_end = 0; info->pos_buf = 0; info->dirty = 0; @@ -108,24 +107,23 @@ RSET rstemp_create( NMEM nmem, const struct key_control *kcontrol, info->temp_path = NULL; else info->temp_path = nmem_strdup(rnew->nmem, temp_path); - rnew->priv=info; + rnew->priv = info; return rnew; } /* rstemp_create */ -static void r_delete (RSET ct) +static void r_delete(RSET ct) { struct rset_temp_info *info = (struct rset_temp_info*) ct->priv; - yaz_log (log_level, "r_delete: set size %ld", (long) info->pos_end); + yaz_log(log_level, "r_delete: set size %ld", (long) info->pos_end); if (info->fname) { - yaz_log (log_level, "r_delete: unlink %s", info->fname); - unlink (info->fname); + yaz_log(log_level, "r_delete: unlink %s", info->fname); + unlink(info->fname); } } - -static RSFD r_open (RSET ct, int flag) +static RSFD r_open(RSET ct, int flag) { struct rset_temp_info *info = (struct rset_temp_info *) ct->priv; RSFD rfd; @@ -134,26 +132,28 @@ static RSFD r_open (RSET ct, int flag) if (info->fd == -1 && info->fname) { if (flag & RSETF_WRITE) - info->fd = open (info->fname, O_BINARY|O_RDWR|O_CREAT, 0666); + info->fd = open(info->fname, O_BINARY|O_RDWR|O_CREAT, 0666); else - info->fd = open (info->fname, O_BINARY|O_RDONLY); + info->fd = open(info->fname, O_BINARY|O_RDONLY); if (info->fd == -1) { - yaz_log (YLOG_FATAL|YLOG_ERRNO, "rstemp: open failed %s", info->fname); - exit (1); + yaz_log(YLOG_FATAL|YLOG_ERRNO, "rstemp: open failed %s", info->fname); + exit(1); } } rfd = rfd_create_base(ct); - if (!rfd->priv){ - prfd= (struct rset_temp_rfd *) nmem_malloc(ct->nmem, sizeof(*prfd)); - rfd->priv=(void *)prfd; - prfd->buf = nmem_malloc (ct->nmem,ct->keycontrol->key_size); - } else + if (!rfd->priv) + { + prfd = (struct rset_temp_rfd *) nmem_malloc(ct->nmem, sizeof(*prfd)); + rfd->priv = (void *)prfd; + prfd->buf = nmem_malloc(ct->nmem,ct->keycontrol->key_size); + } + else prfd= rfd->priv; - r_flush (rfd, 0); + r_flush(rfd, 0); prfd->pos_cur = 0; info->pos_buf = 0; - r_reread (rfd); + r_reread(rfd); prfd->cur = 0; return rfd; } @@ -161,7 +161,7 @@ static RSFD r_open (RSET ct, int flag) /* r_flush: flush current window to file if file is assocated with set */ -static void r_flush (RSFD rfd, int mk) +static void r_flush(RSFD rfd, int mk) { /* struct rset_temp_info *info = ((struct rset_temp_rfd*) rfd)->info; */ struct rset_temp_info *info = rfd->rset->priv; @@ -180,11 +180,11 @@ static void r_flush (RSFD rfd, int mk) if (info->fd == -1) { yaz_log(YLOG_FATAL|YLOG_ERRNO, "rstemp: mkstemp %s", template); - exit (1); + exit(1); } info->fname = nmem_strdup(rfd->rset->nmem, template); #else - char *s = (char*) tempnam (info->temp_path, "zrs"); + char *s = (char*) tempnam(info->temp_path, "zrs"); info->fname= nmem_strdup(rfd->rset->nmem, s); yaz_log(log_level, "creating tempfile %s", info->fname); @@ -192,7 +192,7 @@ static void r_flush (RSFD rfd, int mk) if (info->fd == -1) { yaz_log(YLOG_FATAL|YLOG_ERRNO, "rstemp: open %s", info->fname); - exit (1); + exit(1); } #endif } @@ -201,37 +201,40 @@ static void r_flush (RSFD rfd, int mk) size_t count; int r; - if (lseek (info->fd, info->pos_buf, SEEK_SET) == -1) + if (lseek(info->fd, info->pos_buf, SEEK_SET) == -1) { - yaz_log (YLOG_FATAL|YLOG_ERRNO, "rstemp: lseek %s", info->fname); - exit (1); + yaz_log(YLOG_FATAL|YLOG_ERRNO, "rstemp: lseek (1) %s", info->fname); + exit(1); } count = info->buf_size; if (count > info->pos_end - info->pos_buf) count = info->pos_end - info->pos_buf; - if ((r = write (info->fd, info->buf_mem, count)) < (int) count) + if ((r = write(info->fd, info->buf_mem, count)) < (int) count) { if (r == -1) - yaz_log (YLOG_FATAL|YLOG_ERRNO, "rstemp: write %s", info->fname); + yaz_log(YLOG_FATAL|YLOG_ERRNO, "rstemp: write %s", info->fname); else - yaz_log (YLOG_FATAL, "rstemp: write of %ld but got %ld", + yaz_log(YLOG_FATAL, "rstemp: write of %ld but got %ld", (long) count, (long) r); - exit (1); + exit(1); } info->dirty = 0; } } -static void r_close (RSFD rfd) +static void r_close(RSFD rfd) { /*struct rset_temp_rfd *mrfd = (struct rset_temp_rfd*) rfd->priv; */ struct rset_temp_info *info = (struct rset_temp_info *)rfd->rset->priv; - r_flush (rfd, 0); - if (info->fname && info->fd != -1) + if (rfd_is_last(rfd)) { - close (info->fd); - info->fd = -1; - } /* FIXME - Is this right, don't we risk closing the file too early ?*/ + r_flush(rfd, 0); + if (info->fname && info->fd != -1) + { + close(info->fd); + info->fd = -1; + } + } rfd_delete_base(rfd); } @@ -240,7 +243,7 @@ static void r_close (RSFD rfd) read from file to window if file is assocated with set - indicated by fname */ -static void r_reread (RSFD rfd) +static void r_reread(RSFD rfd) { struct rset_temp_rfd *mrfd = (struct rset_temp_rfd*) rfd->priv; struct rset_temp_info *info = (struct rset_temp_info *)rfd->rset->priv; @@ -257,19 +260,19 @@ static void r_reread (RSFD rfd) count = info->pos_border - info->pos_buf; if (count > 0) { - if (lseek (info->fd, info->pos_buf, SEEK_SET) == -1) + if (lseek(info->fd, info->pos_buf, SEEK_SET) == -1) { - yaz_log (YLOG_FATAL|YLOG_ERRNO, "rstemp: lseek %s", info->fname); - exit (1); + yaz_log(YLOG_FATAL|YLOG_ERRNO, "rstemp: lseek (2) %s fd=%d", info->fname, info->fd); + exit(1); } - if ((r = read (info->fd, info->buf_mem, count)) < (int) count) + if ((r = read(info->fd, info->buf_mem, count)) < (int) count) { if (r == -1) - yaz_log (YLOG_FATAL|YLOG_ERRNO, "rstemp: read %s", info->fname); + yaz_log(YLOG_FATAL|YLOG_ERRNO, "rstemp: read %s", info->fname); else - yaz_log (YLOG_FATAL, "read of %ld but got %ld", + yaz_log(YLOG_FATAL, "read of %ld but got %ld", (long) count, (long) r); - exit (1); + exit(1); } } } @@ -277,8 +280,7 @@ static void r_reread (RSFD rfd) info->pos_border = info->pos_end; } - -static int r_read (RSFD rfd, void *buf, TERMID *term) +static int r_read(RSFD rfd, void *buf, TERMID *term) { struct rset_temp_rfd *mrfd = (struct rset_temp_rfd*) rfd->priv; struct rset_temp_info *info = (struct rset_temp_info *)rfd->rset->priv; @@ -289,21 +291,21 @@ static int r_read (RSFD rfd, void *buf, TERMID *term) { if (nc > info->pos_end) return 0; - r_flush (rfd, 0); + r_flush(rfd, 0); info->pos_buf = mrfd->pos_cur; - r_reread (rfd); + r_reread(rfd); } - memcpy (buf, info->buf_mem + (mrfd->pos_cur - info->pos_buf), + memcpy(buf, info->buf_mem + (mrfd->pos_cur - info->pos_buf), rfd->rset->keycontrol->key_size); if (term) - *term=rfd->rset->term; + *term = rfd->rset->term; /* FIXME - should we store and return terms ?? */ mrfd->pos_cur = nc; mrfd->cur++; return 1; } -static int r_write (RSFD rfd, const void *buf) +static int r_write(RSFD rfd, const void *buf) { struct rset_temp_rfd *mrfd = (struct rset_temp_rfd*) rfd->priv; struct rset_temp_info *info = (struct rset_temp_info *)rfd->rset->priv; @@ -312,13 +314,13 @@ static int r_write (RSFD rfd, const void *buf) if (nc > info->pos_buf + info->buf_size) { - r_flush (rfd, 1); + r_flush(rfd, 1); info->pos_buf = mrfd->pos_cur; if (info->pos_buf < info->pos_end) - r_reread (rfd); + r_reread(rfd); } info->dirty = 1; - memcpy (info->buf_mem + (mrfd->pos_cur - info->pos_buf), buf, + memcpy(info->buf_mem + (mrfd->pos_cur - info->pos_buf), buf, rfd->rset->keycontrol->key_size); mrfd->pos_cur = nc; if (nc > info->pos_end) @@ -327,12 +329,12 @@ static int r_write (RSFD rfd, const void *buf) return 1; } -static void r_pos (RSFD rfd, double *current, double *total) +static void r_pos(RSFD rfd, double *current, double *total) { /* struct rset_temp_rfd *mrfd = (struct rset_temp_rfd*) rfd; */ struct rset_temp_rfd *mrfd = (struct rset_temp_rfd*) rfd->priv; struct rset_temp_info *info = (struct rset_temp_info *)rfd->rset->priv; - *current=(double) mrfd->cur; - *total=(double) info->hits; + *current = (double) mrfd->cur; + *total = (double) info->hits; } -- 1.7.10.4