Skip to content

Commit

Permalink
Add errorcheck in safe mode for Posix threads. Make non-recursive loc…
Browse files Browse the repository at this point in the history
…ks fail when used recursively on Windows. Fix thread pool tests.
  • Loading branch information
lerno committed Sep 30, 2024
1 parent 98fcb61 commit 5bbf6e2
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 20 deletions.
1 change: 1 addition & 0 deletions lib/std/core/runtime.c3
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ fn bool run_tests(TestUnit[] tests)
name.appendf("Testing %s ", unit.name);
name.append_repeat('.', max_name - unit.name.len + 2);
io::printf("%s ", name.str_view());
(void)io::stdout().flush();
if (libc::setjmp(&context.buf) == 0)
{
if (catch err = unit.func())
Expand Down
4 changes: 2 additions & 2 deletions lib/std/os/posix/threads.c3
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import std::thread;
import libc;

const PTHREAD_MUTEX_NORMAL = 0;
const PTHREAD_MUTEX_ERRORCHECK = 1;
const PTHREAD_MUTEX_RECURSIVE = 2;
const PTHREAD_MUTEX_ERRORCHECK = env::LINUX ? 2 : 1;
const PTHREAD_MUTEX_RECURSIVE = env::LINUX ? 1 : 2;

def PosixThreadFn = fn void*(void*);
distinct Pthread_t = void*;
Expand Down
6 changes: 6 additions & 0 deletions lib/std/threads/os/thread_posix.c3
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ fn void! NativeMutex.init(&self, MutexType type)
{
if (posix::pthread_mutexattr_settype(&attr, posix::PTHREAD_MUTEX_RECURSIVE)) return ThreadFault.INIT_FAILED?;
}
else
{
$if env::COMPILER_SAFE_MODE:
if (posix::pthread_mutexattr_settype(&attr, posix::PTHREAD_MUTEX_ERRORCHECK)) return ThreadFault.INIT_FAILED?;
$endif
}
if (posix::pthread_mutex_init(&self.mutex, &attr)) return ThreadFault.INIT_FAILED?;
self.initialized = true;
}
Expand Down
19 changes: 5 additions & 14 deletions lib/std/threads/os/thread_win32.c3
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ fn void! NativeMutex.lock(&mtx)

}
}
if (!mtx.recursive)
if (!mtx.recursive && mtx.already_locked)
{
while (mtx.already_locked) win32::sleep(1);
return ThreadFault.LOCK_FAILED?;
}
mtx.already_locked = true;
}
Expand All @@ -103,20 +103,11 @@ fn void! NativeMutex.lock_timeout(&mtx, ulong ms)
default:
return ThreadFault.LOCK_FAILED?;
}
if (!mtx.recursive)
if (!mtx.recursive && mtx.already_locked)
{
usz left_timeout = ms - 1;
while (mtx.already_locked)
{
if (left_timeout-- == 0)
{
win32::releaseMutex(mtx.handle);
return ThreadFault.LOCK_TIMEOUT?;
}
win32::sleep(1);
}
mtx.already_locked = true;
return ThreadFault.LOCK_FAILED?;
}
mtx.already_locked = true;
}

fn bool NativeMutex.try_lock(&mtx)
Expand Down
2 changes: 2 additions & 0 deletions releasenotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@
- Improved error messages on `Foo a = foo { 1 };` #1496
- Bug in json decoder escape handling.
- Fix bug when reading zip manifest, that would not return a zero terminated string. #1490
- Fix thread tests.
- Detect recursion errors on non-recursive mutexes in safe mode.

### Stdlib changes
- Additional init functions for hashmap.
Expand Down
1 change: 1 addition & 0 deletions test/unit/stdlib/threads/mutex.c3
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import std::os;

const TEST_MAGNITUDE = 10;


fn void! lock_control_test() @test
{
Mutex m;
Expand Down
20 changes: 16 additions & 4 deletions test/unit/stdlib/threads/pool.c3
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fn void! init_destroy() @test

fn void! push_destroy() @test
{
for (usz i = 0; i < 20; i++)
for FOO: (usz i = 0; i < 20; i++)
{
x = 0;
int y = 20;
Expand All @@ -26,9 +26,19 @@ fn void! push_destroy() @test
defer pool.destroy()!!;
work_done.lock()!!;
pool.push(&do_work, &y)!;
work_done.lock()!!;
assert(x == y, "%d: %d != %d", i, x, y);
work_done.unlock()!!;
for (int j = 0; j < 1000; j++)
{
work_done.lock()!!;
if (@atomic_load(x) == @atomic_load(y))
{
(void)work_done.unlock();
break FOO;
}
(void)work_done.unlock();
thread::yield();
}
assert(false, "y never changed");
}
}

Expand All @@ -42,6 +52,7 @@ fn void! push_stop() @test
pool.init()!;
work_done.lock()!!;
pool.push(&do_work, &y)!;
work_done.unlock()!!;
pool.stop_and_destroy()!!;
assert(x == y, "%d: %d != %d", i, x, y);
}
Expand All @@ -56,11 +67,12 @@ fn void startup() @init {
}

fn void shutdown() @finalizer {
work_done.destroy()!!;
(void)work_done.destroy();
}

fn int do_work(void* arg)
{
work_done.lock()!!;
x = *(int*)arg;
work_done.unlock()!!;
return 0;
Expand Down

0 comments on commit 5bbf6e2

Please sign in to comment.