CtdlThreadSchedule() considered harmful. Eliminate. Replaced all
authorArt Cancro <ajc@citadel.org>
Mon, 20 Sep 2010 17:10:19 +0000 (13:10 -0400)
committerArt Cancro <ajc@citadel.org>
Mon, 20 Sep 2010 17:10:19 +0000 (13:10 -0400)
use of it with either dedicated threads or EVT_TIMER hooks.

citadel/context.c
citadel/include/ctdl_module.h
citadel/modules/network/serv_network.c
citadel/modules/rssclient/serv_rssclient.c
citadel/modules/smtp/serv_smtp.c
citadel/threads.c

index aa6033c80889f0ba3abb1e60a4971f761cc9ece0..922f0675db24dda2dd6d56054a09cd09f990bedb 100644 (file)
@@ -437,6 +437,7 @@ void CtdlFillSystemContext(CitContext *context, char *name)
        strcat (sysname, name);
        len = cutuserkey(sysname);
        memcpy(context->curr_user, sysname, len + 1);
+       context->client_socket = (-1);
 
        /* internal_create_user has the side effect of loading the user regardless of wether they
         * already existed or needed to be created
index f1ee1a5f9dec587162ed7ce9acd16b489c30ded2..c0a13c89d5d5a72c43f8bbd8505a325542745647 100644 (file)
@@ -160,7 +160,6 @@ void CtdlModuleStartCryptoMsgs(char *ok_response, char *nosup_response, char *er
  * Citadel Threads API
  */
 struct CtdlThreadNode *CtdlThreadCreate(char *name, long flags, void *(*thread_func) (void *arg), void *args);
-struct CtdlThreadNode *CtdlThreadSchedule(char *name, long flags, void *(*thread_func) (void *arg), void *args, time_t when);
 void CtdlThreadSleep(int secs);
 void CtdlThreadStop(struct CtdlThreadNode *thread);
 int CtdlThreadCheckStop(void);
index 840fb68e0226fc327e35fffc60c4e3cfe57185ce..c7092954208ddd0ec1de6b89ef480c80cb664589 100644 (file)
@@ -2214,15 +2214,10 @@ void create_spool_dirs(void) {
  * 
  * Run through the rooms doing various types of network stuff.
  */
-void *network_do_queue(void *args) {
+void network_do_queue(void) {
        static time_t last_run = 0L;
        struct RoomProcList *ptr;
        int full_processing = 1;
-       struct CitContext networkerCC;
-
-       /* Give the networker its own private CitContext */
-       CtdlFillSystemContext(&networkerCC, "network");
-       citthread_setspecific(MyConKey, (void *)&networkerCC );
 
        /*
         * Run the full set of processing tasks no more frequently
@@ -2242,8 +2237,7 @@ void *network_do_queue(void *args) {
         * with a static variable instead.
         */
        if (doing_queue) {
-               CtdlClearSystemContext();
-               return NULL;
+               return;
        }
        doing_queue = 1;
 
@@ -2318,20 +2312,6 @@ void *network_do_queue(void *args) {
        }
 
        doing_queue = 0;
-
-       /* Reschedule this task to happen again periodically, unless the thread system indicates
-        * that the server is shutting down.
-        */
-       if (!CtdlThreadCheckStop()) {
-               CtdlThreadSchedule("IGnet Network", CTDLTHREAD_BIGSTACK,
-                       network_do_queue, NULL, time(NULL) + 60
-               );
-       }
-       else {
-               CtdlLogPrintf(CTDL_DEBUG, "network: Task STOPPED.\n");
-       }
-       CtdlClearSystemContext();
-       return NULL;
 }
 
 
@@ -2400,6 +2380,24 @@ int network_room_handler (struct ctdlroom *room)
        return 0;
 }
 
+void *ignet_thread(void *arg) {
+       struct CitContext ignet_thread_CC;
+
+       CtdlLogPrintf(CTDL_DEBUG, "ignet_thread() initializing\n");
+       CtdlFillSystemContext(&ignet_thread_CC, "IGnet Queue");
+       citthread_setspecific(MyConKey, (void *)&ignet_thread_CC);
+
+       while (!CtdlThreadCheckStop()) {
+               network_do_queue();
+               CtdlThreadSleep(60);
+       }
+
+       CtdlClearSystemContext();
+       return(NULL);
+}
+
+
+
 
 /*
  * Module entry point
@@ -2413,11 +2411,9 @@ CTDL_MODULE_INIT(network)
                CtdlRegisterProtoHook(cmd_snet, "SNET", "Set network config");
                CtdlRegisterProtoHook(cmd_netp, "NETP", "Identify as network poller");
                CtdlRegisterProtoHook(cmd_nsyn, "NSYN", "Synchronize room to node");
-               CtdlRegisterRoomHook(network_room_handler);
+               CtdlRegisterRoomHook(network_room_handler);
                CtdlRegisterCleanupHook(destroy_network_queue_room);
+               CtdlThreadCreate("SMTP Send", CTDLTHREAD_BIGSTACK, ignet_thread, NULL);
        }
-       else
-               CtdlThreadSchedule("IGnet Network", CTDLTHREAD_BIGSTACK, network_do_queue, NULL, 0);
-       /* return our Subversion id for the Log */
        return "network";
 }
index f59571db14586dd23ec636c45d5c51f3d44c88db..049f8801c7f0d40e834e5b0ac080b18ac2a77885 100644 (file)
@@ -1265,7 +1265,7 @@ void rssclient_scan_room(struct ctdlroom *qrbuf, void *data)
 /*
  * Scan for rooms that have RSS client requests configured
  */
-void *rssclient_scan(void *args) {
+void rssclient_scan(void *args) {
        static time_t last_run = 0L;
        static int doing_rssclient = 0;
        rssnetcfg *rptr = NULL;
@@ -1298,10 +1298,6 @@ void *rssclient_scan(void *args) {
        CtdlLogPrintf(CTDL_DEBUG, "rssclient ended\n");
        last_run = time(NULL);
        doing_rssclient = 0;
-       if (!CtdlThreadCheckStop())
-               CtdlThreadSchedule ("RSS Client", CTDLTHREAD_BIGSTACK, rssclient_scan, NULL, last_run + config.c_net_freq);
-       else
-               CtdlLogPrintf(CTDL_DEBUG, "rssclient: Task STOPPED.\n");
        CtdlClearSystemContext();
        return NULL;
 }
@@ -1312,7 +1308,7 @@ CTDL_MODULE_INIT(rssclient)
        if (threading)
        {
                CtdlLogPrintf(CTDL_INFO, "%s\n", curl_version());
-               CtdlThreadSchedule ("RSS Client", CTDLTHREAD_BIGSTACK, rssclient_scan, NULL, 0);
+               CtdlRegisterSessionHook(rssclient_scan, EVT_TIMER);
        }
 
        StartHandlers = NewHash(1, NULL);
index 69f12fb037bc110485524be5d8039cc3b03e5855..ab01bbd7db9bbc1fb10f4c2049d8be55c49f10a5 100644 (file)
@@ -119,8 +119,6 @@ enum {                              /* Command states for login authentication */
 
 int run_queue_now = 0; /* Set to 1 to ignore SMTP send retry times */
 
-citthread_mutex_t smtp_send_lock;
-
 
 /*****************************************************************************/
 /*                      SMTP SERVER (INBOUND) STUFF                          */
@@ -1732,27 +1730,31 @@ void smtp_do_procmsg(long msgnum, void *userdata) {
 
 
 /*
- * smtp_do_queue()
+ * smtp_queue_thread()
  * 
  * Run through the queue sending out messages.
  */
-void *smtp_do_queue(void *arg) {
+void *smtp_queue_thread(void *arg) {
        int num_processed = 0;
        struct CitContext smtp_queue_CC;
 
        CtdlFillSystemContext(&smtp_queue_CC, "SMTP Send");
-       citthread_setspecific(MyConKey, (void *)&smtp_queue_CC );
-       CtdlLogPrintf(CTDL_INFO, "SMTP client: processing outbound queue\n");
+       citthread_setspecific(MyConKey, (void *)&smtp_queue_CC);
+       CtdlLogPrintf(CTDL_DEBUG, "smtp_queue_thread() initializing\n");
 
-       if (CtdlGetRoom(&CC->room, SMTP_SPOOLOUT_ROOM) != 0) {
-               CtdlLogPrintf(CTDL_ERR, "Cannot find room <%s>\n", SMTP_SPOOLOUT_ROOM);
-       }
-       else {
-               num_processed = CtdlForEachMessage(MSGS_ALL, 0L, NULL, SPOOLMIME, NULL, smtp_do_procmsg, NULL);
-       }
+       while (!CtdlThreadCheckStop()) {
+               
+               CtdlLogPrintf(CTDL_INFO, "SMTP client: processing outbound queue\n");
 
-       citthread_mutex_unlock (&smtp_send_lock);
-       CtdlLogPrintf(CTDL_INFO, "SMTP client: queue run completed; %d messages processed\n", num_processed);
+               if (CtdlGetRoom(&CC->room, SMTP_SPOOLOUT_ROOM) != 0) {
+                       CtdlLogPrintf(CTDL_ERR, "Cannot find room <%s>\n", SMTP_SPOOLOUT_ROOM);
+               }
+               else {
+                       num_processed = CtdlForEachMessage(MSGS_ALL, 0L, NULL, SPOOLMIME, NULL, smtp_do_procmsg, NULL);
+               }
+               CtdlLogPrintf(CTDL_INFO, "SMTP client: queue run completed; %d messages processed\n", num_processed);
+               CtdlThreadSleep(60);
+       }
 
        CtdlClearSystemContext();
        return(NULL);
@@ -1760,38 +1762,6 @@ void *smtp_do_queue(void *arg) {
 
 
 
-/*
- * smtp_queue_thread
- *
- * Create a thread to run the SMTP queue
- *
- * This was created as a response to a situation seen on Uncensored where a bad remote was holding
- * up SMTP sending for long times.
- * Converting to a thread does not fix the problem caused by the bad remote but it does prevent
- * the SMTP sending from stopping housekeeping and the EVT_TIMER event system which in turn prevented
- * other things from happening.
- */
-void smtp_queue_thread (void)
-{
-       if (citthread_mutex_trylock (&smtp_send_lock)) {
-               CtdlLogPrintf(CTDL_DEBUG, "SMTP queue run already in progress\n");
-       }
-       else {
-               CtdlThreadCreate("SMTP Send", CTDLTHREAD_BIGSTACK, smtp_do_queue, NULL);
-       }
-}
-
-
-
-void smtp_server_going_down (void)
-{
-       CtdlLogPrintf(CTDL_DEBUG, "SMTP module clean up for shutdown.\n");
-
-       citthread_mutex_destroy (&smtp_send_lock);
-}
-
-
-
 /*****************************************************************************/
 /*                          SMTP UTILITY COMMANDS                            */
 /*****************************************************************************/
@@ -1924,11 +1894,9 @@ CTDL_MODULE_INIT(smtp)
                                        CitadelServiceSMTP_LMTP_UNF);
 
                smtp_init_spoolout();
-               CtdlRegisterSessionHook(smtp_queue_thread, EVT_TIMER);
                CtdlRegisterSessionHook(smtp_cleanup_function, EVT_STOP);
                CtdlRegisterProtoHook(cmd_smtp, "SMTP", "SMTP utility commands");
-               CtdlRegisterCleanupHook (smtp_server_going_down);
-               citthread_mutex_init (&smtp_send_lock, NULL);
+               CtdlThreadCreate("SMTP Send", CTDLTHREAD_BIGSTACK, smtp_queue_thread, NULL);
        }
        
        /* return our Subversion id for the Log */
index 96d3e4c0492e6cf71e52fc0d0ea7479919438106..ec64ceaefb4407b345ed5858cb010e4f15a093ae 100644 (file)
@@ -1042,68 +1042,6 @@ CtdlThreadNode *CtdlThreadCreate(char *name, long flags, void *(*thread_func) (v
 
 
 
-/*
- * Internal function to schedule a thread.
- * Must be called from within a S_THREAD_LIST critical section
- */ 
-CtdlThreadNode *CtdlThreadSchedule(char *name, long flags, void *(*thread_func) (void *arg), void *args, time_t when)
-{
-       CtdlThreadNode *this_thread;
-
-       if (num_threads >= 32767)
-       {
-               CtdlLogPrintf(CTDL_EMERG, "Thread system. Thread list full.\n");
-               return NULL;
-       }
-               
-       this_thread = malloc(sizeof(CtdlThreadNode));
-       if (this_thread == NULL) {
-               CtdlLogPrintf(CTDL_EMERG, "Thread system, can't allocate CtdlThreadNode, exiting\n");
-               return NULL;
-       }
-       /* Initialise the thread structure */
-       if (ctdl_internal_init_thread_struct(this_thread, flags) == NULL)
-       {
-               free(this_thread);
-               CtdlLogPrintf(CTDL_EMERG, "Thread system, can't initialise CtdlThreadNode, exiting\n");
-               return NULL;
-       }
-
-       /*
-        * If we got here we are going to create the thread so we must initilise the structure
-        * first because most implimentations of threading can't create it in a stopped state
-        * and it might want to do things with its structure that aren't initialised otherwise.
-        */
-       if(name)
-       {
-               this_thread->name = name;
-       }
-       else
-       {
-               this_thread->name = "Un-named Thread";
-       }
-       
-       this_thread->flags = flags;
-       this_thread->thread_func = thread_func;
-       this_thread->user_args = args;
-       
-       /*
-        * When to start this thread
-        */
-       this_thread->when = when;
-
-       begin_critical_section(S_SCHEDULE_LIST);
-       this_thread->next = CtdlThreadSchedList;
-       CtdlThreadSchedList = this_thread;
-       if (this_thread->next)
-               this_thread->next->prev = this_thread;
-       end_critical_section(S_SCHEDULE_LIST);
-       
-       return this_thread;
-}
-
-
-
 CtdlThreadNode *ctdl_thread_internal_start_scheduled (CtdlThreadNode *this_thread)
 {
        int ret = 0;