glib/test/thread-pool-slow: Ensure all unused threads are really stopped

In this tests we wanted to ensure that all the unused threads were
stopped, however while we were calling g_thread_pool_stop_unused_threads
some threads could still be in the process of being recycled even tough
the pool's num_thread values are 0.

In fact, stopping unused threads implies also resetting back the max
unused threads to the previous value, and in this test it caused it to
go from -1 -> 0 and back to -1, after killing the unused threads we
knew about; thus any about-to-be-unused thread that is not killed during
this call will be just left around as a waiting unused thread afterwards.

However, if this function was getting called when a thread was in
between of calling the user function and the moment it was being
recycled (and so when the pool num_threads was updated), but this thread
was not counted in unused_threads, we ended up in having a race because
all the threads were consumed from our POV, but some were actually not
yet unused, and so were kept waiting forever for some new job.

To avoid this in the test, we can ensure that we stop the unused
threads until we the number of them is really 0.

Sadly we need to repeat this as we don't have a clear point in which we
are sure about the fact that our threads are done, while it would be
wrong to stop a thread that is technically not yet marked as unused.

We could also do this in g_thread_pool_stop_unused_threads() itself, but
it would make such function to wait for threads to complete, and this is
probably not what was expected in the initial API.

Fixes: #2685
This commit is contained in:
Marco Trevisan (Treviño) 2022-07-11 18:17:41 +02:00
parent a5ccaa0525
commit fabdc2d4fa

View File

@ -61,47 +61,45 @@ test_thread_functions (void)
g_assert_cmpint (g_thread_pool_get_max_idle_time (), ==, 0);
}
static void
thread_wait_func (gpointer data,
gpointer user_data)
{
guint timeout_ms = GPOINTER_TO_UINT (data);
guint *n_threads_executed = user_data;
g_usleep (timeout_ms);
g_atomic_int_inc (n_threads_executed);
}
static void
test_thread_stop_unused (void)
{
GThreadPool *pool;
guint i;
guint limit = 100;
guint n_threads_executed = 0;
/* Spawn a few threads. */
g_thread_pool_set_max_unused_threads (-1);
pool = g_thread_pool_new (thread_wait_func, &n_threads_executed, -1, FALSE, NULL);
pool = g_thread_pool_new ((GFunc) g_usleep, NULL, -1, FALSE, NULL);
for (i = 0; i < limit; i++)
g_thread_pool_push (pool, GUINT_TO_POINTER (1000), NULL);
/* Wait for the threads to migrate. */
while ((guint) g_atomic_int_get (&n_threads_executed) < limit)
while (g_thread_pool_get_num_threads (pool) != 0)
g_usleep (100);
g_thread_pool_stop_unused_threads ();
g_assert_cmpuint (g_thread_pool_get_num_threads (pool), ==, 0);
/* Wait for threads to die. */
while (g_thread_pool_get_num_unused_threads () != 0)
do {
/* We may need to repeat this in case we tried to stop unused threads
* while some thread was still active, and not yet marked as non-used,
* despite what g_thread_pool_get_num_threads() tells us.
* And if this happens the thread will be kept in the unused queue
* indefinitely, so we need to stop it again, until we're really done.
*/
g_thread_pool_stop_unused_threads ();
g_usleep (100);
} while (g_thread_pool_get_num_unused_threads () != 0);
g_assert_cmpint (g_thread_pool_get_num_unused_threads (), ==, 0);
g_assert_cmpuint (g_thread_pool_get_num_unused_threads (), ==, 0);
g_thread_pool_set_max_unused_threads (MAX_THREADS);
g_assert_cmpuint (g_thread_pool_get_num_threads (pool), ==, 0);
g_assert_cmpuint (g_thread_pool_get_num_unused_threads (), ==, 0);
g_thread_pool_free (pool, FALSE, TRUE);
}