Skip to content

Commit 6940ca8

Browse files
committed
Rework startup lock using atomic variable + waitpid(WNOHANG).
Turns out you cannot lock mutex in thread A and then let thread B to unlock it with POSIX threads.
1 parent fef5f53 commit 6940ca8

File tree

2 files changed

+32
-26
lines changed

2 files changed

+32
-26
lines changed

pt.c

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
*/
2121

2222
#include <sys/types.h>
23+
#include <sys/wait.h>
2324
#include <unistd.h>
2425
#include <stdio.h>
2526

@@ -181,9 +182,6 @@ void set_proc_attrs(const char *fmt, ...)
181182

182183
/* pid */
183184
pt[process_no].pid=getpid();
184-
185-
/* for sure the process is running */
186-
pt[process_no].flags |= OSS_PROC_IS_RUNNING;
187185
}
188186

189187

@@ -229,9 +227,11 @@ void reset_process_slot( int p_id )
229227
}
230228

231229

230+
enum {CHLD_STARTING, CHLD_OK, CHLD_FAILED};
231+
232232
static __attribute__((__noreturn__)) void child_startup_failed(void)
233233
{
234-
lock_release(&pt[process_no].startup_lock);
234+
atomic_store(&pt[process_no].startup_result, CHLD_FAILED);
235235
exit(1);
236236
}
237237

@@ -288,18 +288,12 @@ int internal_fork(const char *proc_desc, unsigned int flags,
288288
}
289289

290290
pt[new_idx].pid = 0;
291-
pt[new_idx].startup_result = -1;
292291

293-
if (lock_init(&pt[new_idx].startup_lock) == NULL) {
294-
LM_CRIT("failed to init startup lock for process %d", new_idx);
295-
return -1;
296-
}
297-
lock_get(&pt[new_idx].startup_lock);
292+
atomic_init(&pt[new_idx].startup_result, CHLD_STARTING);
298293

299294
if ( (pid=fork())<0 ){
300295
LM_CRIT("cannot fork \"%s\" process (%d: %s)\n",proc_desc,
301296
errno, strerror(errno));
302-
lock_destroy(&pt[new_idx].startup_lock);
303297
reset_process_slot( new_idx );
304298
return -1;
305299
}
@@ -344,25 +338,37 @@ int internal_fork(const char *proc_desc, unsigned int flags,
344338
free_route_lists(sroutes);
345339
sroutes = NULL;
346340
}
347-
pt[process_no].startup_result = 0;
348-
lock_release(&pt[process_no].startup_lock);
341+
atomic_store(&pt[process_no].startup_result, CHLD_OK);
349342
return 0;
350343
}else{
351344
/* parent process */
352-
/* wait for the child to complete the start-up */
353-
lock_get(&pt[new_idx].startup_lock);
354-
lock_destroy(&pt[new_idx].startup_lock);
355-
if (pt[new_idx].startup_result != 0) {
356-
LM_CRIT("failed to initialize child process %d\n", new_idx);
357-
reset_process_slot( new_idx );
358-
return -1;
345+
/* wait for the child to complete the critical sectoin of the
346+
* start-up */
347+
while (atomic_load(&pt[new_idx].startup_result) == CHLD_STARTING) {
348+
int status;
349+
pid_t result = waitpid(pid, &status, WNOHANG);
350+
if (result < 0) {
351+
if (errno == EINTR)
352+
continue;
353+
goto child_is_down;
354+
}
355+
if (result == 0) {
356+
// Child has not exited yet
357+
continue;
358+
}
359+
// Child has exited, oops
360+
goto child_is_down;
359361
}
360-
/* Do not set PID for child in the main process. Let the child do
361-
* that as this will act as a marker to tell us that the init
362-
* sequance of the child proc was completed.
363-
* pt[new_idx].pid = pid; */
362+
if (atomic_load(&pt[new_idx].startup_result) != CHLD_OK) {
363+
goto child_is_down;
364+
}
365+
pt[new_idx].flags |= OSS_PROC_IS_RUNNING;
364366
tcp_connect_proc_to_tcp_main( new_idx, 0);
365367
return new_idx;
368+
child_is_down:
369+
LM_CRIT("failed to initialize child process %d\n", new_idx);
370+
reset_process_slot( new_idx );
371+
return -1;
366372
}
367373
}
368374

pt.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include <sys/types.h>
2929
#include <unistd.h>
3030

31+
#include "atomic.h"
3132
#include "pt_load.h"
3233

3334
#define MAX_PT_DESC 128
@@ -91,8 +92,7 @@ struct process_table {
9192
struct proc_load_info load;
9293

9394
/* synchronization during fork */
94-
gen_lock_t startup_lock;
95-
int startup_result;
95+
atomic_t startup_result;
9696
};
9797

9898

0 commit comments

Comments
 (0)