From b136962546e20c701fce2904484ba1f999312281 Mon Sep 17 00:00:00 2001 From: Art Cancro Date: Fri, 18 Mar 2005 21:25:10 +0000 Subject: [PATCH] * Finished removing all the "dynamic session data" stuff in order to boost reliability, improve performance, and reduce complexity. --- citadel/ChangeLog | 41 +++++++------- citadel/citserver.c | 119 ++++------------------------------------ citadel/citserver.h | 4 -- citadel/msgbase.c | 67 +++++++++++++--------- citadel/serv_calendar.c | 7 ++- citadel/serv_calendar.h | 2 +- citadel/serv_vcard.c | 33 ++++------- citadel/server.h | 26 +-------- citadel/sysdep.c | 7 +-- 9 files changed, 95 insertions(+), 211 deletions(-) diff --git a/citadel/ChangeLog b/citadel/ChangeLog index b01230426..972fda327 100644 --- a/citadel/ChangeLog +++ b/citadel/ChangeLog @@ -1,4 +1,8 @@ $Log$ + Revision 641.27 2005/03/18 21:25:06 ajc + * Finished removing all the "dynamic session data" stuff in order to + boost reliability, improve performance, and reduce complexity. + Revision 641.26 2005/03/12 05:42:35 ajc * Trying to fix a memory bug somewhere. * While working on the above, noticed that the way we did the per-session @@ -731,7 +735,7 @@ Revision 621.6 2004/06/03 02:49:14 ajc * html.c: allow parsing of tags even when they're qualified - (i.e. instead of just ) + (i.e. instead of just ) * html.c: handle escaped decimal characters (such as ' for an apostrophe) Revision 621.5 2004/06/03 02:28:16 ajc @@ -1866,11 +1870,11 @@ Revision 605.45 2003/05/02 04:02:47 ajc * setup.c: allow specification of the Citadel system account by either - username or uid + username or uid * setup.c: tell init to re-read /etc/inittab by sending a SIGHUP to pid 1 - instead of hunting around for the correct init or telinit command + instead of hunting around for the correct init or telinit command * docs/citadel.html: documented the above change, and also rewrote some - other stuff to be less BBS-specific + other stuff to be less BBS-specific Revision 605.44 2003/04/30 16:16:13 ajc * Minor fix to ESMTP greeting (missing '-' screwed up pipelining) @@ -4559,9 +4563,9 @@ Revision 572.23 2000/08/09 17:14:34 ajc msgbase.c: fixed a bug in - remove_any_whitespace_to_the_left_or_right_of_at_symbol() that was - causing the eply function to fail on names with whitespace in - certain parts of the string. This closes Bug #56. + remove_any_whitespace_to_the_left_or_right_of_at_symbol() that was + causing the eply function to fail on names with whitespace in + certain parts of the string. This closes Bug #56. Revision 572.22 2000/08/05 04:24:00 ajc * Added [idle] to client wholist display for sessions idle >15 minutes @@ -4821,7 +4825,7 @@ Revision 1.485 2000/03/11 20:26:03 ajc Revision 1.484 2000/03/11 19:22:19 nbryant * commands.c: improved timing of background keepalives if connection is - lagged + lagged Revision 1.483 2000/03/11 05:08:48 nbryant * commands.c: oops, that mutex stuff wasn't necessary @@ -4979,7 +4983,7 @@ Revision 1.446 2000/01/17 17:09:23 ajc Revision 1.445 2000/01/17 05:38:14 ajc * citserver.c: cleanup hook functions are now run under the proper context, - even when initiated by the housekeeper thread + even when initiated by the housekeeper thread * serv_pop3.c: establish a place to hold the message list Revision 1.444 2000/01/17 04:26:39 ajc @@ -5260,7 +5264,7 @@ Revision 1.375 1999/09/29 21:13:17 ajc Revision 1.374 1999/09/29 17:26:56 ajc * serv_vcard.c: fixed crashola bug in cmd_greg() * tools.c: simplified and improved the string tokenizer. Now it runs in a - single pass with no intermediate buffer. + single pass with no intermediate buffer. Revision 1.373 1999/09/28 03:27:37 ajc * Fully migrated cmd_greg() and cmd_regi() into serv_vcard (still has bugs) @@ -5797,8 +5801,8 @@ Tue Jan 12 22:30:00 EST 1999 Art Cancro - Defined format type 4 for MIME - msgbase.c: *temporary* hacks in output_message() for Type 4 - citmail.c: added more robust header parsing, and support - for Type 4. Also eliminated the crappy built-in - SMTP server. + for Type 4. Also eliminated the crappy built-in + SMTP server. - Updated some of the technical documentation Sun Jan 10 13:34:36 EST 1999 Art Cancro @@ -6451,8 +6455,8 @@ Mon Aug 24 23:45:01 EDT 1998 Art Cancro * citadelapi.c: Added CtdlForEachRoom() function Mon Aug 24 20:04:04 EDT 1998 Nathan Bryant - * Makefile.in: new target `cleaner' does the same as `realclean' - without removing sysdep.h + * Makefile.in: new target `cleaner' does the same as `realclean' + without removing sysdep.h * proto.h: is bad. eliminate. I've moved the prototypes into several header files, one per .c file @@ -6532,15 +6536,12 @@ Sun Jul 19 17:26:12 EDT 1998 Nathan Bryant * .cvsignore: added userlist Sun Jul 12 20:58:59 EDT 1998 Art Cancro - * Finished migrating everything to the new data store. - * Replaced the binary "calllog" with the ASCII "citadel.log" - * Began converting broken utilities that depend on the old data store + * Finished migrating everything to the new data store. + * Replaced the binary "calllog" with the ASCII "citadel.log" + * Began converting broken utilities that depend on the old data store Sat Jul 11 00:20:48 EDT 1998 Nathan Bryant * Makefile.in: removed msgstats Fri Jul 10 1998 Art Cancro * Initial CVS import - - - diff --git a/citadel/citserver.c b/citadel/citserver.c index ad95f0a82..edc26d9d4 100644 --- a/citadel/citserver.c +++ b/citadel/citserver.c @@ -168,27 +168,6 @@ void master_cleanup(int exitcode) { } -/* - * Free any per-session data allocated by modules or whatever - */ -void deallocate_user_data(struct CitContext *con) -{ - struct CtdlSessData *ptr; - int i; - - while (con->FirstSessData != NULL) { - lprintf(CTDL_DEBUG, "Deallocating user data symbol #%d\n", i++); - if (con->FirstSessData->sym_data != NULL) { - free(con->FirstSessData->sym_data); - } - ptr = con->FirstSessData->next; - free(con->FirstSessData); - con->FirstSessData = ptr; - } -} - - - /* * Terminate a session and remove its context data structure. @@ -220,10 +199,8 @@ void RemoveContext (struct CitContext *con) end_critical_section(S_SESSION_TABLE); /* Run any cleanup routines registered by loadable modules. - * Note 1: This must occur *before* deallocate_user_data() because the - * cleanup functions might touch dynamic session data. - * Note 2: We have to "become_session()" because the cleanup functions - * might make references to "CC" assuming it's the right one. + * Note: We have to "become_session()" because the cleanup functions + * might make references to "CC" assuming it's the right one. */ become_session(con); PerformSessionHooks(EVT_STOP); @@ -236,9 +213,6 @@ void RemoveContext (struct CitContext *con) unlink(con->temp); lprintf(CTDL_NOTICE, "[%3d] Session ended.\n", con->cs_pid); - /* Deallocate any user-data attached to this session */ - deallocate_user_data(con); - /* If the client is still connected, blow 'em away. */ lprintf(CTDL_DEBUG, "Closing socket %d\n", con->client_socket); close(con->client_socket); @@ -255,81 +229,6 @@ void RemoveContext (struct CitContext *con) -/* - * Return a pointer to some generic per-session user data. - * (This function returns NULL if the requested symbol is not allocated.) - * - * NOTE: we use critical sections for allocating and de-allocating these, - * but not for locating one. - */ -void *CtdlGetUserData(unsigned long requested_sym) -{ - struct CtdlSessData *ptr; - - for (ptr = CC->FirstSessData; ptr != NULL; ptr = ptr->next) - if (ptr->sym_id == requested_sym) - return(ptr->sym_data); - - lprintf(CTDL_ERR, "ERROR! CtdlGetUserData(%ld) symbol not allocated\n", - requested_sym); - return NULL; -} - - -/* - * Allocate some generic per-session user data. - */ -void CtdlAllocUserData(unsigned long requested_sym, size_t num_bytes) -{ - struct CtdlSessData *ptr; - - lprintf(CTDL_DEBUG, "CtdlAllocUserData(%ld) called\n", requested_sym); - - /* Fail silently if the symbol is already registered. */ - for (ptr = CC->FirstSessData; ptr != NULL; ptr = ptr->next) { - if (ptr->sym_id == requested_sym) { - return; - } - } - - /* Grab us some memory! Dem's good eatin' !! */ - ptr = malloc(sizeof(struct CtdlSessData)); - ptr->sym_id = requested_sym; - ptr->sym_data = malloc(num_bytes); - memset(ptr->sym_data, 0, num_bytes); - - begin_critical_section(S_SESSION_TABLE); - ptr->next = CC->FirstSessData; - CC->FirstSessData = ptr; - end_critical_section(S_SESSION_TABLE); - - lprintf(CTDL_DEBUG, "CtdlAllocUserData(%ld) finished\n", requested_sym); -} - - -/* - * Change the size of a buffer allocated with CtdlAllocUserData() - */ -void CtdlReallocUserData(unsigned long requested_sym, size_t num_bytes) -{ - struct CtdlSessData *ptr; - - for (ptr = CC->FirstSessData; ptr != NULL; ptr = ptr->next) { - if (ptr->sym_id == requested_sym) { - ptr->sym_data = realloc(ptr->sym_data, num_bytes); - return; - } - } - - lprintf(CTDL_ERR, "CtdlReallocUserData() ERROR: symbol %ld not found!\n", - requested_sym); -} - - - - - - /* * cmd_info() - tell the client about this server */ @@ -776,7 +675,8 @@ void cmd_ipgm(char *argbuf) */ if (!CC->is_local_socket) { sleep(5); - cprintf("%d Authentication failed.\n", ERROR + PASSWORD_REQUIRED); + cprintf("%d Authentication failed.\n", + ERROR + PASSWORD_REQUIRED); } else if (secret == config.c_ipgm_secret) { CC->internal_pgm = 1; @@ -786,15 +686,21 @@ void cmd_ipgm(char *argbuf) } else { sleep(5); - cprintf("%d Authentication failed.\n", ERROR + PASSWORD_REQUIRED); + cprintf("%d Authentication failed.\n", + ERROR + PASSWORD_REQUIRED); lprintf(CTDL_ERR, "Warning: ipgm authentication failed.\n"); CC->kill_me = 1; } - /* Now change the ipgm secret for the next round. */ + /* Now change the ipgm secret for the next round. + * (Disabled because it breaks concurrent scripts. The fact that + * we no longer accept IPGM over the network should be sufficient + * to prevent brute-force attacks. If you don't agree, uncomment + * this block.) get_config(); config.c_ipgm_secret = rand(); put_config(); + */ } @@ -905,7 +811,6 @@ void begin_session(struct CitContext *con) con->cs_flags = 0; con->upload_type = UPL_FILE; con->dl_is_net = 0; - con->FirstSessData = NULL; con->nologin = 0; if ((config.c_maxsessions > 0)&&(num_sessions > config.c_maxsessions)) diff --git a/citadel/citserver.h b/citadel/citserver.h index b5d27a243..dd599cb3e 100644 --- a/citadel/citserver.h +++ b/citadel/citserver.h @@ -29,10 +29,6 @@ void cmd_ipgm (char *argbuf); void cmd_down (void); void cmd_scdn (char *argbuf); void cmd_extn (char *argbuf); -void deallocate_user_data(struct CitContext *con); -void *CtdlGetUserData(unsigned long requested_sym); -void CtdlAllocUserData(unsigned long requested_sym, size_t num_bytes); -void CtdlReallocUserData(unsigned long requested_sym, size_t num_bytes); void do_command_loop(void); void do_async_loop(void); void begin_session(struct CitContext *con); diff --git a/citadel/msgbase.c b/citadel/msgbase.c index 2aaf88e23..ed2973be3 100644 --- a/citadel/msgbase.c +++ b/citadel/msgbase.c @@ -51,9 +51,6 @@ #include "genstamp.h" #include "internet_addressing.h" -#define desired_section ((char *)CtdlGetUserData(SYM_DESIRED_SECTION)) -#define ma ((struct ma_info *)CtdlGetUserData(SYM_MA_INFO)) - extern struct config config; long config_msgnum; @@ -762,7 +759,7 @@ void mime_download(char *name, char *filename, char *partnum, char *disp, return; /* ...or if this is not the desired section */ - if (strcasecmp(desired_section, partnum)) + if (strcasecmp(CC->download_desired_section, partnum)) return; CC->download_fp = tmpfile(); @@ -915,12 +912,15 @@ void fixed_output_pre(char *name, char *filename, char *partnum, char *disp, void *content, char *cbtype, size_t length, char *encoding, void *cbuserdata) { - lprintf(CTDL_DEBUG, "fixed_output_pre() type=<%s>\n", cbtype); - if (!strcasecmp(cbtype, "multipart/alternative")) { - ++ma->is_ma; - ma->did_print = 0; - return; - } + struct ma_info *ma; + + ma = (struct ma_info *)cbuserdata; + lprintf(CTDL_DEBUG, "fixed_output_pre() type=<%s>\n", cbtype); + if (!strcasecmp(cbtype, "multipart/alternative")) { + ++ma->is_ma; + ma->did_print = 0; + return; + } } /* @@ -930,12 +930,15 @@ void fixed_output_post(char *name, char *filename, char *partnum, char *disp, void *content, char *cbtype, size_t length, char *encoding, void *cbuserdata) { - lprintf(CTDL_DEBUG, "fixed_output_post() type=<%s>\n", cbtype); - if (!strcasecmp(cbtype, "multipart/alternative")) { - --ma->is_ma; - ma->did_print = 0; - return; - } + struct ma_info *ma; + + ma = (struct ma_info *)cbuserdata; + lprintf(CTDL_DEBUG, "fixed_output_post() type=<%s>\n", cbtype); + if (!strcasecmp(cbtype, "multipart/alternative")) { + --ma->is_ma; + ma->did_print = 0; + return; + } } /* @@ -948,6 +951,9 @@ void fixed_output(char *name, char *filename, char *partnum, char *disp, char *ptr; char *wptr; size_t wlen; + struct ma_info *ma; + + ma = (struct ma_info *)cbuserdata; lprintf(CTDL_DEBUG, "fixed_output() type=<%s>\n", cbtype); @@ -997,6 +1003,9 @@ void choose_preferred(char *name, char *filename, char *partnum, char *disp, { char buf[SIZ]; int i; + struct ma_info *ma; + + ma = (struct ma_info *)cbuserdata; if (ma->is_ma > 0) { for (i=0; ipreferred_formats, '|'); ++i) { @@ -1019,6 +1028,9 @@ void output_preferred(char *name, char *filename, char *partnum, char *disp, char buf[SIZ]; int add_newline = 0; char *text_content; + struct ma_info *ma; + + ma = (struct ma_info *)cbuserdata; /* This is not the MIME part you're looking for... */ if (strcasecmp(partnum, ma->chosen_part)) return; @@ -1132,6 +1144,7 @@ int CtdlOutputPreLoadedMsg( char *nl; /* newline string */ int suppress_f = 0; int subject_found = 0; + struct ma_info *ma; /* buffers needed for RFC822 translation */ char suser[SIZ]; @@ -1170,9 +1183,10 @@ int CtdlOutputPreLoadedMsg( } else { /* Parse the message text component */ mptr = TheMessage->cm_fields['M']; - mime_parser(mptr, NULL, - *mime_download, NULL, NULL, - NULL, 0); + ma = malloc(sizeof(struct ma_info)); + memset(ma, 0, sizeof(struct ma_info)); + mime_parser(mptr, NULL, *mime_download, NULL, NULL, (void *)ma, 0); + free(ma); /* If there's no file open by this time, the requested * section wasn't found, so print an error */ @@ -1180,7 +1194,7 @@ int CtdlOutputPreLoadedMsg( if (do_proto) cprintf( "%d Section %s not found.\n", ERROR + FILE_NOT_FOUND, - desired_section); + CC->download_desired_section); } } return((CC->download_fp != NULL) ? om_ok : om_mime_error); @@ -1465,16 +1479,17 @@ START_TEXT: * we use will display those parts as-is. */ if (TheMessage->cm_format_type == FMT_RFC822) { - CtdlAllocUserData(SYM_MA_INFO, sizeof(struct ma_info)); - memset(ma, 0, sizeof(struct ma_info)); if (mode == MT_MIME) { + ma = malloc(sizeof(struct ma_info)); + memset(ma, 0, sizeof(struct ma_info)); strcpy(ma->chosen_part, "1"); mime_parser(mptr, NULL, *choose_preferred, *fixed_output_pre, - *fixed_output_post, NULL, 0); + *fixed_output_post, (void *)ma, 0); mime_parser(mptr, NULL, - *output_preferred, NULL, NULL, NULL, 0); + *output_preferred, NULL, NULL, (void *)ma, 0); + free(ma); } else { mime_parser(mptr, NULL, @@ -1591,11 +1606,11 @@ void cmd_msgp(char *cmdbuf) void cmd_opna(char *cmdbuf) { long msgid; - - CtdlAllocUserData(SYM_DESIRED_SECTION, SIZ); + char desired_section[SIZ]; msgid = extract_long(cmdbuf, 0); extract(desired_section, cmdbuf, 1); + safestrncpy(CC->download_desired_section, desired_section, sizeof CC->download_desired_section); CtdlOutputMsg(msgid, MT_DOWNLOAD, 0, 1, 1); } diff --git a/citadel/serv_calendar.c b/citadel/serv_calendar.c index 65ec59f7a..7d28b9403 100644 --- a/citadel/serv_calendar.c +++ b/citadel/serv_calendar.c @@ -1883,10 +1883,14 @@ int ical_obj_aftersave(struct CtdlMessage *msg) void ical_session_startup(void) { - CtdlAllocUserData(SYM_CIT_ICAL, sizeof(struct cit_ical)); + CIT_ICAL = malloc(sizeof(struct cit_ical)); memset(CIT_ICAL, 0, sizeof(struct cit_ical)); } +void ical_session_shutdown(void) { + free(CIT_ICAL); +} + #endif /* CITADEL_WITH_CALENDAR_SERVICE */ @@ -1901,6 +1905,7 @@ char *serv_calendar_init(void) CtdlRegisterSessionHook(ical_create_room, EVT_LOGIN); CtdlRegisterProtoHook(cmd_ical, "ICAL", "Citadel iCal commands"); CtdlRegisterSessionHook(ical_session_startup, EVT_START); + CtdlRegisterSessionHook(ical_session_shutdown, EVT_STOP); #endif return "$Id$"; } diff --git a/citadel/serv_calendar.h b/citadel/serv_calendar.h index c12b1408a..c58a6e40d 100644 --- a/citadel/serv_calendar.h +++ b/citadel/serv_calendar.h @@ -19,7 +19,7 @@ struct cit_ical { int avoid_sending_invitations; }; -#define CIT_ICAL ((struct cit_ical *)CtdlGetUserData(SYM_CIT_ICAL)) +#define CIT_ICAL CC->CIT_ICAL /* * When saving a message containing calendar information, we keep track of diff --git a/citadel/serv_vcard.c b/citadel/serv_vcard.c index 8500c9c19..5dd95868f 100644 --- a/citadel/serv_vcard.c +++ b/citadel/serv_vcard.c @@ -65,14 +65,6 @@ #include "vcard.h" #include "serv_ldap.h" -struct vcard_internal_info { - long msgnum; -}; - -/* Message number symbol used internally by these functions */ -#define VC ((struct vcard_internal_info *)CtdlGetUserData(SYM_VCARD)) - - /* * set global flag calling for an aide to validate new users */ @@ -488,8 +480,11 @@ int vcard_upload_aftersave(struct CtdlMessage *msg) { /* * back end function used for callbacks */ -void vcard_gu_backend(long msgnum, void *userdata) { - VC->msgnum = msgnum; +void vcard_gu_backend(long supplied_msgnum, void *userdata) { + long *msgnum; + + msgnum = (long *) userdata; + *msgnum = supplied_msgnum; } @@ -502,6 +497,7 @@ struct vCard *vcard_get_user(struct ctdluser *u) { char config_rm[ROOMNAMELEN]; struct CtdlMessage *msg; struct vCard *v; + long VCmsgnum; strcpy(hold_rm, CC->room.QRname); /* save current room */ MailboxName(config_rm, sizeof config_rm, u, USERCONFIGROOM); @@ -512,14 +508,14 @@ struct vCard *vcard_get_user(struct ctdluser *u) { } /* We want the last (and probably only) vcard in this room */ - VC->msgnum = (-1); + VCmsgnum = (-1); CtdlForEachMessage(MSGS_LAST, 1, "text/x-vcard", - NULL, vcard_gu_backend, NULL); + NULL, vcard_gu_backend, (void *)&VCmsgnum ); getroom(&CC->room, hold_rm); /* return to saved room */ - if (VC->msgnum < 0L) return vcard_new(); + if (VCmsgnum < 0L) return vcard_new(); - msg = CtdlFetchMessage(VC->msgnum, 1); + msg = CtdlFetchMessage(VCmsgnum, 1); if (msg == NULL) return vcard_new(); v = vcard_load(msg->cm_fields['M']); @@ -946,14 +942,6 @@ void vcard_create_room(void) -/* - * Session startup, allocate some per-session data - */ -void vcard_session_startup_hook(void) { - CtdlAllocUserData(SYM_VCARD, sizeof(struct vcard_internal_info)); -} - - /* * When a user logs in... */ @@ -974,7 +962,6 @@ char *serv_vcard_init(void) { struct ctdlroom qr; - CtdlRegisterSessionHook(vcard_session_startup_hook, EVT_START); CtdlRegisterSessionHook(vcard_session_login_hook, EVT_LOGIN); CtdlRegisterMessageHook(vcard_upload_beforesave, EVT_BEFORESAVE); CtdlRegisterMessageHook(vcard_upload_aftersave, EVT_AFTERSAVE); diff --git a/citadel/server.h b/citadel/server.h index caa545363..32362acf7 100644 --- a/citadel/server.h +++ b/citadel/server.h @@ -31,28 +31,6 @@ struct CtdlMessage { #define CM_SKIP_HOOKS 0x01 /* Don't run server-side handlers */ -/* - * Generic per-session variable or data structure storage - */ -struct CtdlSessData { - struct CtdlSessData *next; - unsigned long sym_id; - void *sym_data; -}; - -/* - * Static user data symbol types. Server extensions can ask for dynamic - * extensions to per-session data, but the symbol ID has to be listed here. - */ -enum { - SYM_DESIRED_SECTION, /* Used by the MIME parser */ - SYM_MA_INFO, /* Handles multipart/alternative */ - SYM_CIT_ICAL, /* Used by the calendar service */ - SYM_VCARD, /* vCard handling requires this */ - SYM_MAX -}; - - /* * Here's the big one... the Citadel context structure. * @@ -106,6 +84,7 @@ struct CitContext { char cs_inet_email[SIZ];/* Return address of outbound Internet mail */ FILE *download_fp; /* Fields relating to file transfer */ + char download_desired_section[128]; FILE *upload_fp; char upl_file[PATH_MAX]; char upl_path[PATH_MAX]; @@ -145,12 +124,13 @@ struct CitContext { char preferred_formats[SIZ]; /* Preferred MIME formats */ /* Dynamically allocated session data */ - struct CtdlSessData *FirstSessData; struct citimap *IMAP; struct citpop3 *POP3; struct citsmtp *SMTP; char *SMTP_RECPS; char *SMTP_ROOMS; + struct cit_ical *CIT_ICAL; /* calendaring data */ + struct ma_info *ma; /* multipart/alternative data */ }; typedef struct CitContext t_context; diff --git a/citadel/sysdep.c b/citadel/sysdep.c index 23acaa4be..b571b8b47 100644 --- a/citadel/sysdep.c +++ b/citadel/sysdep.c @@ -1004,12 +1004,7 @@ do_select: force_purge = 0; if (FD_ISSET(serviceptr->msock, &readfds)) { ssock = accept(serviceptr->msock, NULL, 0); - if (ssock < 0) { - lprintf(CTDL_CRIT, - "citserver: accept(): %s\n", - strerror(errno)); - } - else { + if (ssock >= 0) { lprintf(CTDL_DEBUG, "New client socket %d\n", ssock); -- 2.30.2