merge master into uploadcmdfix
diff --git a/ChangeLog b/ChangeLog
index 6b635a5..381c7c6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,14 @@
* src/mod_rivet_ng/rivetCore.c: update comments to upload command
* tests: changed upload file test
+2020-04-02 Massimo Manghi <mxmanghi@apache.org>
+ * doc/xml/formbroker.xml: fixed examples
+ * src/mod_rivet_ng/rivet_lazy_mpm.c: resolved access to shared counter
+ that was causing segfaults when child process was terminated
+
+2020-01-19 Massimo Manghi <mxmanghi@apache.org>
+ * doc/xml/commands.xml: remove entry for command incr0 (dropped in 3.0)
+
2019-12-30 Massimo Manghi <mxmanghi@apache.org>
* src/mod_rivet_ng/rivetCore.c: fixed bug in 'upload filename' that
caused segfaults instead of Tcl errors when called without arguments
diff --git a/doc/rivet.css b/doc/rivet.css
index 71ae239..c407a8c 100644
--- a/doc/rivet.css
+++ b/doc/rivet.css
@@ -31,7 +31,7 @@
font-family: monospace;
white-space: pre;
width: 95%;
- background-color: #f7e9c6;
+ background-color: #e8ecf2;
border: solid;
color: #000000;
border-color: #a3b1bc;
@@ -83,8 +83,8 @@
{
COLOR: #ffffff ;
font-style: italic;
- border: solid 3px #e8a34b;
- background-color: #593c27;
+ border: solid 3px #1d252b;
+ background-color: #a3b1bc;
PADDING: 0.5em;
}
@@ -139,7 +139,7 @@
font-weight: bold;
margin-top: 10px;
color: #ffffff ;
- background-color: #f7e9c6;
+ background-color: #e8ecf2;
border: solid 1px #a3b1bc;
padding: 1px
}
@@ -154,15 +154,15 @@
font-size: 12px;
}
-/*/*/
-
+/*
A{
color: maroon;
}
-a:visited {
+A:visited {
color: darkgreen;
}
+*/
BODY P {
@@ -272,7 +272,7 @@
DIV.NAVFOOTER {
color: #000000;
- background-color: #f7e9c6;
+ background-color: #e8ecf2;
padding: 5px;
margin-top: 10px;
width: 100%;
@@ -281,7 +281,7 @@
DIV.NUKEFOOTER {
color: #000000;
- background-color: #f7e9c6;
+ background-color: #e8ecf2;
padding: 5px;
margin-top: 10px;
width: 100%;
@@ -290,7 +290,7 @@
DIV.NAVHEADER {
color: #000000;
- background-color: #f7e9c6;
+ background-color: #e8ecf2;
padding: 5px;
margin-bottom: 10px;
width: 100%;
@@ -306,7 +306,7 @@
padding-left: 10px;
padding-right: 10px;
color: #000000;
- background-color: #f7e9c6;
+ background-color: #e8ecf2;
}
DIV.EXAMPLE {
border: thin dotted #22AA22;
@@ -323,8 +323,8 @@
/* list-style: url("images/tux-bullet.png") disc; */
}
-table.namespaces {
- border-collapse: collapse;
+.namespaces {
+ border: 1px solid black;
}
.namespaces td {
@@ -333,14 +333,11 @@
}
.namespaces thead {
- background-color: #5e3c27;
- color: white;
- font-size: 1em;
- text-decoration: none;
+ background-color: #aaf;
}
.namespaces tr.init {
- background-color: #bb8766;
+ background-color: #e88;
}
.namespaces tr.childinit {
@@ -348,7 +345,7 @@
}
.namespaces tr.processing {
- background-color: #bb8766;
+ background-color: #e88;
}
.namespaces tr.childexit {
@@ -358,10 +355,8 @@
table.directives {
border-collapse: collapse;
}
-
.directives thead {
- background-color: #5e3c27;
- color: white;
+ background-color: #bbf;
font-size: 1em;
text-decoration: none;
}
@@ -371,30 +366,29 @@
}
.directives tbody tr > :first-child {
- background-color: #bb8766;
- color: white;
+ background-color: #eee;
padding-left: 2em;
padding-right: 2em;
text-align: left;
}
.directives tbody tr > :last-child {
- text-decoration: none;
- font-weight: normal;
- font-size: small;
- border-left: 1px solid black;
- text-align: left;
+ text-decoration: none;
+ font-weight: normal;
+ font-size: small;
+ border-left: 1px solid black;
+ text-align: left;
}
.directives tbody tr td {
text-align: center;
text-decoration: none;
font-weight: normal;
- padding-left: 1em;
- padding-right: 1em;
- padding-bottom: 4px;
- padding-top: 4px;
- border-bottom: 1px solid black;
+ padding-left: 1em;
+ padding-right: 1em;
+ padding-bottom: 4px;
+ padding-top: 4px;
+ border-bottom: 1px solid black;
}
.note td {
diff --git a/doc/xml/commands.xml b/doc/xml/commands.xml
index ddbf976..0b95309 100644
--- a/doc/xml/commands.xml
+++ b/doc/xml/commands.xml
@@ -953,38 +953,6 @@
</refsect1>
</refentry>
- <refentry id="incr0">
- <refnamediv>
- <refname>incr0</refname>
- <refpurpose>increment a variable or set it to 1 if nonexistent.</refpurpose>
- </refnamediv>
-
- <refsynopsisdiv>
- <cmdsynopsis>
- <command>incr0</command>
- <arg><replaceable>varname</replaceable></arg>
- <arg><replaceable>num</replaceable></arg>
- </cmdsynopsis>
- </refsynopsisdiv>
- <refsect1>
- <title>Description</title>
- <para>
- Increment a variable
- <option><replaceable>varname</replaceable></option> by
- <option><replaceable>num</replaceable></option>. If the
- variable doesn't exist, create it instead of returning an
- error.
- </para>
- <note>
- incr0 functionality is provided by the native <command>incr</command> in
- Tcl >= 8.5, therefore this command is deprecated and kept as an
- interpreter alias only for compatibility. As such <command>incr0</command>
- wasn't moved to the ::rivet namespace and
- it will be removed in future versions of Rivet.
- </note>
- </refsect1>
- </refentry>
-
<refentry id="inspect">
<refnamediv>
<refname>inspect</refname>
diff --git a/doc/xml/formbroker.xml b/doc/xml/formbroker.xml
index 3160f2b..6ea64ab 100644
--- a/doc/xml/formbroker.xml
+++ b/doc/xml/formbroker.xml
@@ -127,7 +127,8 @@
Example of form data validation (assuming ::rivet::load_response is loading the array <emphasis>response</emphasis>
with data taken from a form non displayed here)
</para>
- <programlisting>% set fbroker [::FormBroker create {var1 integer} {var2 unsigned} {var3 string} {var4 integer bounds {-10 100}}]
+ <programlisting>% package require formbroker
+% set fbroker [::FormBroker create {var1 integer} {var2 unsigned} {var3 string} {var4 integer bounds {-10 100}}]
::FormBroker::form0
% ::rivet::load_response
@@ -243,10 +244,11 @@
the 'constrain' attribute forced the truncation of the string.
In fact this applies also to the integer and unsigned values
</para>
-<programlisting>% set fbroker [::FormBroker create {var1 integer bounds 10 constrain} \
- {var2 unsigned constrain} \
- {var3 string length 10 constrain} \
- {var4 integer bounds {-10 100} constrain}]
+<programlisting>% package require formbroker
+% set fbroker [::FormBroker create {var1 integer bounds 10 constrain} \
+ {var2 unsigned constrain} \
+ {var3 string length 10 constrain} \
+ {var4 integer bounds {-10 100} constrain}]
::FormBroker::form0
% ::rivet::load_response
% parray response
@@ -287,10 +289,11 @@
such is a way to create form default arrays to initialize forms created with
the <xref linkend="form_package">form</xref> package.
</para>
-<programlisting>set fbroker [::FormBroker create {var1 integer default 0} \
- {var2 unsigned default 1} \
- {var3 string} \
- {var4 integer default 0}]
+<programlisting>
+set fbroker [::FormBroker create {var1 integer default 0} \
+ {var2 unsigned default 1} \
+ {var3 string} \
+ {var4 integer default 0}]
% $fbroker response a
% parray a
a(var1) = 0
diff --git a/src/mod_rivet_ng/rivet_lazy_mpm.c b/src/mod_rivet_ng/rivet_lazy_mpm.c
index f5bc6df..40f68b6 100644
--- a/src/mod_rivet_ng/rivet_lazy_mpm.c
+++ b/src/mod_rivet_ng/rivet_lazy_mpm.c
@@ -58,13 +58,12 @@
request_rec* r;
int ctype;
int ap_sts;
- rivet_server_conf* conf; /* rivet_server_conf* record */
+ rivet_server_conf* conf; /* rivet_server_conf* record */
} lazy_tcl_worker;
/* virtual host thread queue descriptor */
typedef struct vhost_iface {
- int idle_threads_cnt; /* idle threads for the virtual hosts */
int threads_count; /* total number of running and idle threads */
apr_thread_mutex_t* mutex; /* mutex protecting 'array' */
apr_array_header_t* array; /* LIFO array of lazy_tcl_worker pointers */
@@ -192,14 +191,21 @@
Lazy_RunConfScript(private,w,child_init);
+ idx = w->conf->idx;
+
+ /* After the thread has run the configuration script we
+ increment the threads counter */
+
+ apr_thread_mutex_lock(module_globals->mpm->vhosts[idx].mutex);
+ (module_globals->mpm->vhosts[idx].threads_count)++;
+ apr_thread_mutex_unlock(module_globals->mpm->vhosts[idx].mutex);
+
/* The thread is now set up to serve request within the the
* do...while loop controlled by private->keep_going */
- idx = w->conf->idx;
apr_thread_mutex_lock(w->mutex);
do
{
- module_globals->mpm->vhosts[idx].idle_threads_cnt++;
while ((w->status != init) && (w->status != thread_exit)) {
apr_thread_cond_wait(w->condition,w->mutex);
}
@@ -209,7 +215,6 @@
}
w->status = processing;
- module_globals->mpm->vhosts[idx].idle_threads_cnt--;
/* Content generation */
@@ -236,8 +241,8 @@
} while (private->ext->keep_going);
apr_thread_mutex_unlock(w->mutex);
- ap_log_error(APLOG_MARK,APLOG_DEBUG,APR_SUCCESS,w->server,"processor thread orderly exit");
Lazy_RunConfScript(private,w,child_exit);
+ ap_log_error(APLOG_MARK,APLOG_DEBUG,APR_SUCCESS,w->server,"processor thread orderly exit");
apr_thread_mutex_lock(module_globals->mpm->vhosts[idx].mutex);
(module_globals->mpm->vhosts[idx].threads_count)--;
@@ -260,7 +265,7 @@
static lazy_tcl_worker* create_worker (apr_pool_t* pool,server_rec* server)
{
- lazy_tcl_worker* w;
+ lazy_tcl_worker* w;
w = apr_pcalloc(pool,sizeof(lazy_tcl_worker));
@@ -319,19 +324,18 @@
for (s = root_server; s != NULL; s = s->next)
{
- int vh;
+ int idx;
apr_array_header_t* array;
rivet_server_conf* rsc = RIVET_SERVER_CONF(s->module_config);
- vh = rsc->idx;
- rv = apr_thread_mutex_create(&module_globals->mpm->vhosts[vh].mutex,
+ idx = rsc->idx;
+ rv = apr_thread_mutex_create(&module_globals->mpm->vhosts[idx].mutex,
APR_THREAD_MUTEX_UNNESTED,pool);
ap_assert(rv == APR_SUCCESS);
array = apr_array_make(pool,0,sizeof(void*));
ap_assert(array != NULL);
- module_globals->mpm->vhosts[vh].array = array;
- module_globals->mpm->vhosts[vh].idle_threads_cnt = 0;
- module_globals->mpm->vhosts[vh].threads_count = 0;
+ module_globals->mpm->vhosts[idx].array = array;
+ module_globals->mpm->vhosts[idx].threads_count = 0;
}
module_globals->mpm->server_shutdown = 0;
}
@@ -377,7 +381,7 @@
if (apr_is_empty_array(array))
{
w = create_worker(module_globals->pool,r->server);
- (module_globals->mpm->vhosts[conf->idx].threads_count)++;
+ //(module_globals->mpm->vhosts[conf->idx].threads_count)++;
}
else
{
@@ -385,7 +389,9 @@
}
apr_thread_mutex_unlock(mutex);
-
+
+ /* Locking the thread descriptor structure mutex */
+
apr_thread_mutex_lock(w->mutex);
w->r = r;
w->ctype = ctype;
@@ -421,27 +427,42 @@
return private->ext->interp;
}
+/*
+ * -- LazyBridge_Finalize
+ *
+ * Bridge thread and resources shutdown
+ *
+ */
+
apr_status_t LazyBridge_Finalize (void* data)
{
- int vh;
- rivet_server_conf* conf = RIVET_SERVER_CONF(((server_rec*) data)->module_config);
+ int idx;
+ server_rec* server = (server_rec*) data;
+ rivet_server_conf* conf = RIVET_SERVER_CONF(server->module_config);
module_globals->mpm->server_shutdown = 1;
- for (vh = 0; vh < module_globals->vhosts_count; vh++)
+ for (idx = 0; idx < module_globals->vhosts_count; idx++)
{
int try;
int count;
apr_array_header_t* array;
apr_thread_mutex_t* mutex;
- mutex = module_globals->mpm->vhosts[vh].mutex;
- array = module_globals->mpm->vhosts[vh].array;
- apr_thread_mutex_lock(mutex);
- try = 0;
- do {
+ mutex = module_globals->mpm->vhosts[idx].mutex;
+ array = module_globals->mpm->vhosts[idx].array;
- count = module_globals->mpm->vhosts[vh].threads_count;
- if (((conf->idx == vh) && (count == 1)) || (count == 0)) { break; }
+ /* we need to lock the vhost data mutex */
+
+ apr_thread_mutex_lock(mutex);
+
+ count = module_globals->mpm->vhosts[idx].threads_count;
+ apr_thread_mutex_unlock(mutex);
+ try = 0;
+
+ while ((try++ < 3) && (count > 0))
+ {
+ ap_log_error(APLOG_MARK,APLOG_DEBUG,APR_SUCCESS,server,"waiting for %d thread to exit",count);
+ if ((conf->idx == idx) && (count == 1)) { break; }
while (!apr_is_empty_array(array))
{
@@ -449,20 +470,31 @@
w = *(lazy_tcl_worker**) apr_array_pop(array);
apr_thread_mutex_lock(w->mutex);
- w->r = NULL;
- w->status = thread_exit;
+ w->r = NULL;
+ w->status = thread_exit;
apr_thread_cond_signal(w->condition);
apr_thread_mutex_unlock(w->mutex);
}
- apr_sleep(10000);
+
+ apr_thread_mutex_lock(mutex);
+ count = module_globals->mpm->vhosts[idx].threads_count;
+ apr_thread_mutex_unlock(mutex);
- } while ((try++ < 3));
- apr_thread_mutex_unlock(mutex);
+ apr_sleep(1000);
+ }
}
return APR_SUCCESS;
}
+/*
+ * -- LazyBridge_ExitHandler
+ *
+ *
+ *
+ */
+
+
int LazyBridge_ExitHandler(rivet_thread_private* private)
{
@@ -473,29 +505,6 @@
private->ext->keep_going = 0;
- /*
- * This is the only place where exit_command and
- * exit_command_status are set, anywere alse these
- * fields are only read. We lock on writing to synchronize
- * with other threads that might try to access
- * this info. That means that in the unlikely case
- * of several threads calling ::rivet::exit
- * simultaneously the first sets the exit code.
- * This is just terrible, it highlights the bad habit
- * of calling 'exit' when programming with mod_rivet
- * and calls out for a version of Tcl with which
- * we could safely call Tcl_DeleteInterp and then terminate
- * a single thread
-
- apr_thread_mutex_lock(module_globals->mpm->mutex);
- if (module_globals->mpm->exit_command == 0)
- {
- module_globals->mpm->exit_command = 1;
- module_globals->mpm->exit_command_status = private->exit_status;
- }
- apr_thread_mutex_unlock(module_globals->mpm->mutex);
- */
-
if (!private->running_conf->single_thread_exit)
{
/* We now tell the supervisor to terminate the Tcl worker thread pool
@@ -509,51 +518,6 @@
return TCL_OK;
}
-#if 0
-int LazyBridge_ExitHandler(rivet_thread_private* private)
-{
-
- /* This is not strictly necessary, because this command will
- * eventually terminate the whole processes */
-
- /* This will force the current thread to exit */
-
- private->ext->keep_going = 0;
-
- /*
- * This is the only place where exit_command and
- * exit_command_status are set, anywere alse these
- * fields are only read. We lock on writing to synchronize
- * with other threads that might try to access
- * this info. That means that in the unlikely case
- * of several threads calling ::rivet::exit
- * simultaneously the first sets the exit code.
- * This is just terrible, it highlights the bad habit
- * of calling 'exit' when programming with mod_rivet
- * and calls out for a version of Tcl with which
- * we could safely call Tcl_DeleteInterp and then terminate
- * a single thread
- */
-
- apr_thread_mutex_lock(module_globals->mpm->mutex);
- if (module_globals->mpm->exit_command == 0)
- {
- module_globals->mpm->exit_command = 1;
- module_globals->mpm->exit_command_status = private->exit_status;
- }
- apr_thread_mutex_unlock(module_globals->mpm->mutex);
-
- if (private->running_conf->single_thread_exit) return TCL_OK;
-
- /* We now tell the supervisor to terminate the Tcl worker thread pool
- * to exit and is sequence the whole process to shutdown
- * by calling exit() */
-
- LazyBridge_Finalize (private->r->server);
- return TCL_OK;
-}
-#endif
-
DLLEXPORT
RIVET_MPM_BRIDGE {
NULL,
