Skip to content

Commit 938ee19

Browse files
fix a crash at application shutdown where in some situations we could deinitialize the protected allocator mutex while a lane was still using it.
1 parent 6ac65b8 commit 938ee19

File tree

1 file changed

+45
-20
lines changed

1 file changed

+45
-20
lines changed

src/lanes.c

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ static void lane_cleanup( struct s_lane* s)
326326
#endif // THREADWAIT_METHOD == THREADWAIT_CONDVAR
327327

328328
#if HAVE_LANE_TRACKING
329-
if( tracking_first)
329+
if( tracking_first != NULL)
330330
{
331331
// Lane was cleaned up, no need to handle at process termination
332332
tracking_remove( s);
@@ -452,17 +452,17 @@ LUAG_FUNC( linda_send)
452452

453453
// change status of lane to "waiting"
454454
{
455-
struct s_lane *s;
455+
struct s_lane* s;
456456
enum e_status prev_status = ERROR_ST; // prevent 'might be used uninitialized' warnings
457-
STACK_GROW(L, 1);
457+
STACK_GROW( L, 1);
458458

459459
STACK_CHECK( L);
460460
lua_pushlightuserdata( L, CANCEL_TEST_KEY);
461461
lua_rawget( L, LUA_REGISTRYINDEX);
462462
s = lua_touserdata( L, -1); // lightuserdata (true 's_lane' pointer) or nil if in the main Lua state
463-
lua_pop(L, 1);
463+
lua_pop( L, 1);
464464
STACK_END( L, 0);
465-
if( s)
465+
if( s != NULL)
466466
{
467467
prev_status = s->status; // RUNNING, most likely
468468
ASSERT_L( prev_status == RUNNING); // but check, just in case
@@ -473,14 +473,14 @@ LUAG_FUNC( linda_send)
473473
// could not send because no room: wait until some data was read before trying again, or until timeout is reached
474474
if( !SIGNAL_WAIT( &linda->read_happened, &K->lock_, timeout))
475475
{
476-
if( s)
476+
if( s != NULL)
477477
{
478478
s->waiting_on = NULL;
479479
s->status = prev_status;
480480
}
481481
break;
482482
}
483-
if( s)
483+
if( s != NULL)
484484
{
485485
s->waiting_on = NULL;
486486
s->status = prev_status;
@@ -621,7 +621,7 @@ LUAG_FUNC( linda_receive)
621621
s = lua_touserdata( L, -1); // lightuserdata (true 's_lane' pointer) or nil if in the main Lua state
622622
lua_pop( L, 1);
623623
STACK_END( L, 0);
624-
if( s)
624+
if( s != NULL)
625625
{
626626
prev_status = s->status; // RUNNING, most likely
627627
ASSERT_L( prev_status == RUNNING); // but check, just in case
@@ -632,14 +632,14 @@ LUAG_FUNC( linda_receive)
632632
// not enough data to read: wakeup when data was sent, or when timeout is reached
633633
if( !SIGNAL_WAIT( &linda->write_happened, &K->lock_, timeout))
634634
{
635-
if( s)
635+
if( s != NULL)
636636
{
637637
s->waiting_on = NULL;
638638
s->status = prev_status;
639639
}
640640
break;
641641
}
642-
if( s)
642+
if( s != NULL)
643643
{
644644
s->waiting_on = NULL;
645645
s->status = prev_status;
@@ -837,7 +837,7 @@ static int linda_tostring( lua_State* L, int _idx, bool_t _opt)
837837
{
838838
luaL_argcheck( L, linda, _idx, "expected a linda object!");
839839
}
840-
if( linda)
840+
if( linda != NULL)
841841
{
842842
char text[32];
843843
int len;
@@ -1245,6 +1245,10 @@ static MUTEX_T selfdestruct_cs;
12451245

12461246
struct s_lane* volatile selfdestruct_first = SELFDESTRUCT_END;
12471247

1248+
// After a lane has removed itself from the chain, it still performs some processing.
1249+
// The terminal desinit sequence should wait for all such processing to terminate before force-killing threads
1250+
int volatile selfdestructing_count = 0;
1251+
12481252
/*
12491253
* Add the lane to selfdestruct chain; the ones still running at the end of the
12501254
* whole process will be cancelled.
@@ -1280,6 +1284,8 @@ static bool_t selfdestruct_remove( struct s_lane *s )
12801284
if (*ref == s) {
12811285
*ref= s->selfdestruct_next;
12821286
s->selfdestruct_next= NULL;
1287+
// the terminal shutdown should wait until the lane is done with its lua_close()
1288+
++ selfdestructing_count;
12831289
found= TRUE;
12841290
break;
12851291
}
@@ -1325,10 +1331,10 @@ static int selfdestruct_gc( lua_State* L)
13251331
{
13261332
// Signal _all_ still running threads to exit (including the timer thread)
13271333
//
1328-
MUTEX_LOCK( &selfdestruct_cs );
1334+
MUTEX_LOCK( &selfdestruct_cs);
13291335
{
13301336
struct s_lane* s = selfdestruct_first;
1331-
while( s != SELFDESTRUCT_END )
1337+
while( s != SELFDESTRUCT_END)
13321338
{
13331339
// attempt a regular unforced hard cancel with a small timeout
13341340
bool_t cancelled = THREAD_ISNULL( s->thread) || thread_cancel( s, 0.0001, FALSE);
@@ -1344,7 +1350,7 @@ static int selfdestruct_gc( lua_State* L)
13441350
s = s->selfdestruct_next;
13451351
}
13461352
}
1347-
MUTEX_UNLOCK( &selfdestruct_cs );
1353+
MUTEX_UNLOCK( &selfdestruct_cs);
13481354

13491355
// When noticing their cancel, the lanes will remove themselves from
13501356
// the selfdestruct chain.
@@ -1384,7 +1390,7 @@ static int selfdestruct_gc( lua_State* L)
13841390
MUTEX_UNLOCK( &selfdestruct_cs);
13851391
// if timeout elapsed, or we know all threads have acted, stop waiting
13861392
t_now = now_secs();
1387-
if( n == 0 || ( t_now >= t_until))
1393+
if( n == 0 || (t_now >= t_until))
13881394
{
13891395
DEBUGSPEW_CODE( fprintf( stderr, "%d uncancelled lane(s) remain after waiting %fs at process end.\n", n, shutdown_timeout - (t_until - t_now)));
13901396
break;
@@ -1393,6 +1399,19 @@ static int selfdestruct_gc( lua_State* L)
13931399
}
13941400
}
13951401

1402+
// If some lanes are currently cleaning after themselves, wait until they are done.
1403+
// They are no longer listed in the selfdestruct chain, but they still have to lua_close().
1404+
{
1405+
bool_t again = TRUE;
1406+
do
1407+
{
1408+
MUTEX_LOCK( &selfdestruct_cs);
1409+
again = (selfdestructing_count > 0) ? TRUE : FALSE;
1410+
MUTEX_UNLOCK( &selfdestruct_cs);
1411+
YIELD();
1412+
} while( again);
1413+
}
1414+
13961415
//---
13971416
// Kill the still free running threads
13981417
//
@@ -1404,7 +1423,7 @@ static int selfdestruct_gc( lua_State* L)
14041423
// these are not running, and the state can be closed
14051424
MUTEX_LOCK( &selfdestruct_cs);
14061425
{
1407-
struct s_lane* s= selfdestruct_first;
1426+
struct s_lane* s = selfdestruct_first;
14081427
while( s != SELFDESTRUCT_END)
14091428
{
14101429
struct s_lane* next_s = s->selfdestruct_next;
@@ -1830,10 +1849,14 @@ static THREAD_RETURN_T THREAD_CALLCONV lane_main( void *vs)
18301849
{
18311850
// We're a free-running thread and no-one's there to clean us up.
18321851
//
1833-
lua_close( s->L );
1852+
lua_close( s->L);
18341853
s->L = L = 0;
18351854

18361855
lane_cleanup( s);
1856+
MUTEX_LOCK( &selfdestruct_cs);
1857+
// done with lua_close(), terminal shutdown sequence may proceed
1858+
-- selfdestructing_count;
1859+
MUTEX_UNLOCK( &selfdestruct_cs);
18371860
}
18381861
else
18391862
{
@@ -1920,7 +1943,7 @@ LUAG_FUNC( thread_new)
19201943
DEBUGSPEW_CODE( fprintf( stderr, INDENT_BEGIN "thread_new: setup\n" INDENT_END));
19211944
DEBUGSPEW_CODE( ++ debugspew_indent_depth);
19221945

1923-
// populate with selected libraries at the same time
1946+
// populate with selected libraries at the same time
19241947
//
19251948
L2 = luaG_newstate( L, on_state_create, libs);
19261949

@@ -1935,7 +1958,7 @@ LUAG_FUNC( thread_new)
19351958

19361959
DEBUGSPEW_CODE( fprintf( stderr, INDENT_BEGIN "thread_new: update 'package'\n" INDENT_END));
19371960
// package
1938-
if( package)
1961+
if( package != 0)
19391962
{
19401963
luaG_inter_copy_package( L, L2, package, eLM_LaneBody);
19411964
}
@@ -1944,7 +1967,7 @@ LUAG_FUNC( thread_new)
19441967

19451968
STACK_CHECK( L);
19461969
STACK_CHECK( L2);
1947-
if( required)
1970+
if( required != 0)
19481971
{
19491972
int nbRequired = 1;
19501973
DEBUGSPEW_CODE( fprintf( stderr, INDENT_BEGIN "thread_new: require 'required' list\n" INDENT_END));
@@ -2068,7 +2091,9 @@ LUAG_FUNC( thread_new)
20682091
res = luaG_inter_copy( L, L2, args, eLM_LaneBody); // L->L2
20692092
DEBUGSPEW_CODE( -- debugspew_indent_depth);
20702093
if( res != 0)
2094+
{
20712095
return luaL_error( L, "tried to copy unsupported types");
2096+
}
20722097
}
20732098
STACK_MID( L, 0);
20742099

0 commit comments

Comments
 (0)