Skip to content

Commit

Permalink
Merge openenclave#4325
Browse files Browse the repository at this point in the history 
4325: Revise thread_interrupt test r=mingweishih a=mingweishih

The PR revises the thread_interrupt test and avoids the flakiness in the blocking test by ensuring the SIGUSR1 is unregistered after the test with registered signal.

Signed-off-by: Ming-Wei Shih <[email protected]>

Co-authored-by: Ming-Wei Shih <[email protected]>
  • Loading branch information
@oeciteam @mingweishih
oeciteam and mingweishih committed Dec 6, 2021
2 parents d006e51 + 2eba152 commit f88a272
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 49 deletions.
24 changes: 24 additions & 0 deletions include/openenclave/internal/tests.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,30 @@ OE_EXTERNC_BEGIN

#define OE_TEST_IGNORE(COND) OE_TEST_IF(COND, false)

#define OE_EXPECT_IF(VALUE, EXPECTED, OE_TEST_ABORT) \
do \
{ \
uint64_t value = (uint64_t)(VALUE); \
uint64_t expected = (uint64_t)(EXPECTED); \
if (value != expected) \
{ \
OE_PRINT( \
STDERR, \
"Test failed: %s(%u): %s expected: %lu, got: %lu\n", \
__FILE__, \
__LINE__, \
__FUNCTION__, \
expected, \
value); \
if (OE_TEST_ABORT) \
OE_ABORT(); \
} \
} while (0)

#define OE_EXPECT(VALUE, EXPECTED) OE_EXPECT_IF(VALUE, EXPECTED, true)

#define OE_EXPECT_IGNORE(VALUE, EXPECTED) OE_EXPECT_IF(VALUE, EXPECTED, false)

/* Multi-threaded test cases may need a specific base allocation
* when using allocators that make use of thread-local storage, such
* as tcmalloc or snmalloc.
Expand Down
18 changes: 0 additions & 18 deletions tests/sgx/td_state/enc/enc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,6 @@
#include <stdio.h>
#include <stdlib.h>

#define OE_EXPECT(a, b) \
do \
{ \
uint64_t value = (uint64_t)(a); \
uint64_t expected = (uint64_t)(b); \
if (value != expected) \
{ \
printf( \
"Test failed: %s(%u): %s expected: %lu, got: %lu\n", \
__FILE__, \
__LINE__, \
__FUNCTION__, \
expected, \
value); \
oe_abort(); \
} \
} while (0)

typedef struct _thread_info_t
{
int tid;
Expand Down
51 changes: 24 additions & 27 deletions tests/sgx/thread_interrupt/enc/enc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ void enc_run_thread_nonblocking(int tid)

void enc_thread_interrupt_nonblocking(void)
{
oe_result_t result;
int tid = 0;

host_get_tid(&tid);
Expand All @@ -144,9 +143,7 @@ void enc_thread_interrupt_nonblocking(void)
// Test interrupting a non-blocking thread
printf("(tid=%d) Create a non-blocking thread...\n", tid);

result = host_create_thread(0 /* blocking */);
if (result != OE_OK)
return;
OE_TEST(host_create_thread(0 /* blocking */) == OE_OK);

while (__atomic_load_n(&_thread_info_nonblocking.lock, __ATOMIC_ACQUIRE) !=
1)
Expand Down Expand Up @@ -236,7 +233,9 @@ uint64_t thread_terminate_handler(oe_exception_record_t* exception_record)
{
int self_tid = 0;

OE_TEST(exception_record->code == OE_EXCEPTION_UNKNOWN);
OE_EXPECT(exception_record->code, OE_EXCEPTION_UNKNOWN);

OE_EXPECT(exception_record->host_signal_number, SIGUSR2);

host_get_tid(&self_tid);
OE_TEST(_thread_info_blocking.tid == self_tid);
Expand Down Expand Up @@ -288,9 +287,8 @@ void enc_run_thread_blocking(int tid)
return;
}

void enc_thread_interrupt_blocking(int* ret)
void enc_thread_interrupt_blocking()
{
oe_result_t result;
int tid = 0;
int retry = 0;

Expand All @@ -299,9 +297,8 @@ void enc_thread_interrupt_blocking(int* ret)

// Test interrupting a blocking thread
printf("(tid=%d) Create a blocking thread...\n", tid);
result = host_create_thread(1 /* blocking */);
if (result != OE_OK)
return;

OE_TEST(host_create_thread(1 /* blocking */) == OE_OK);

while (!_thread_info_blocking.lock)
{
Expand All @@ -312,13 +309,13 @@ void enc_thread_interrupt_blocking(int* ret)

host_sleep_msec(30);

OE_TEST(
oe_sgx_td_register_host_signal(_thread_info_blocking.td, SIGUSR1) ==
true);

_handler_entered = 0;
while (!_handler_entered)
{
if (oe_sgx_td_register_host_signal(_thread_info_blocking.td, SIGUSR1) !=
true)
break;

printf(
"(tid=%d) Sending registered signal (SIGUSR1) to (td=0x%lx, "
"tid=%d)...%d\n",
Expand All @@ -327,8 +324,7 @@ void enc_thread_interrupt_blocking(int* ret)
_thread_info_blocking.tid,
++retry);

if (_thread_info_blocking.td->state != OE_TD_STATE_RUNNING)
break;
OE_EXPECT(_thread_info_blocking.td->state, OE_TD_STATE_RUNNING);

host_send_interrupt(_thread_info_blocking.tid, SIGUSR1);

Expand All @@ -343,16 +339,23 @@ void enc_thread_interrupt_blocking(int* ret)
host_sleep_msec(30);
}

if (retry != 10)
goto done;
OE_TEST(retry == 10);

retry = 0;
_handler_entered = 0;

OE_TEST(
oe_sgx_td_unregister_host_signal(_thread_info_blocking.td, SIGUSR1) ==
true);

// The oe_sgx_td_unmask_host_signal API is usually expected to be used
// by the thread itself. Use here for testing purposes.
oe_sgx_td_unmask_host_signal(_thread_info_blocking.td);

OE_TEST(
oe_sgx_td_host_signal_registered(_thread_info_blocking.td, SIGUSR2) ==
false);

while (!_handler_entered)
{
printf(
Expand All @@ -363,8 +366,7 @@ void enc_thread_interrupt_blocking(int* ret)
_thread_info_blocking.tid,
++retry);

if (_thread_info_blocking.td->state != OE_TD_STATE_RUNNING)
break;
OE_EXPECT(_thread_info_blocking.td->state, OE_TD_STATE_RUNNING);

host_send_interrupt(_thread_info_blocking.tid, SIGUSR2);

Expand All @@ -379,20 +381,15 @@ void enc_thread_interrupt_blocking(int* ret)
host_sleep_msec(30);
}

if (retry != 10)
goto done;
OE_TEST(retry == 10);

*ret = 1;

done:
/* unblock and shutdown the thread */
OE_TEST(
oe_sgx_td_register_host_signal(_thread_info_blocking.td, SIGUSR2) ==
true);

printf(
"(tid=%d) Shutting down (td=0x%lx, "
"tid=%d)\n",
"(tid=%d) Shutting down (td=0x%lx, tid=%d)\n",
tid,
(uint64_t)_thread_info_blocking.td,
_thread_info_blocking.tid);
Expand Down
4 changes: 1 addition & 3 deletions tests/sgx/thread_interrupt/host/host.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,7 @@ int main(int argc, const char* argv[])
}
else if (!strcmp(argv[2], "blocking"))
{
int ret = 0;
result = enc_thread_interrupt_blocking(enclave, &ret);
OE_TEST(ret == 1);
result = enc_thread_interrupt_blocking(enclave);
if (result != OE_OK)
oe_put_err("oe_call_enclave() failed: result=%u", result);
OE_TEST(oe_terminate_enclave(enclave) == OE_OK);
Expand Down
2 changes: 1 addition & 1 deletion tests/sgx/thread_interrupt/thread_interrupt.edl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ enclave {

trusted {
public void enc_thread_interrupt_nonblocking();
public void enc_thread_interrupt_blocking([in, out] int* ret);
public void enc_thread_interrupt_blocking();
public void enc_run_thread_nonblocking(int tid);
public void enc_run_thread_blocking(int tid);
};
Expand Down

0 comments on commit f88a272

Please sign in to comment.