Skip to content

Commit bc73bf9

Browse files
committed
Forward port changes from v0.6 release branch
Merge a number of fixes and improvements from release-0.6 into main, such as use signals for link/unlink/monitor/demonitor (#1555) and fix handling of filenames in stack traces (#1556).
2 parents 6941851 + bc21d17 commit bc73bf9

File tree

16 files changed

+781
-280
lines changed

16 files changed

+781
-280
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ and a race condition in otp_socket code
7979
- Fixed a bug where calling repeatedly `process_info` on a stopped process could cause an out of
8080
memory error
8181
- Fixed possible concurrency problems in ESP32 UART driver
82+
- Fixed concurrency and memory leak related to links and monitors
83+
- Fixed issues with parsing of line references for stack traces
8284

8385
### Changed
8486

src/libAtomVM/context.c

Lines changed: 118 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
#define DEFAULT_STACK_SIZE 8
5353
#define BYTES_PER_TERM (TERM_BITS / 8)
5454

55-
static struct ResourceMonitor *context_monitors_handle_terminate(Context *ctx);
55+
static struct Monitor *context_monitors_handle_terminate(Context *ctx);
5656
static void destroy_extended_registers(Context *ctx, unsigned int live);
5757

5858
Context *context_new(GlobalContext *glb)
@@ -133,27 +133,54 @@ void context_destroy(Context *ctx)
133133
// Ensure process is not registered
134134
globalcontext_maybe_unregister_process_id(ctx->global, ctx->process_id);
135135

136+
// Process any link/unlink/monitor/demonitor signal that arrived recently
137+
// Also process ProcessInfoRequestSignal so caller isn't trapped waiting
138+
MailboxMessage *signal_message = mailbox_process_outer_list(&ctx->mailbox);
139+
while (signal_message) {
140+
if (signal_message->type == ProcessInfoRequestSignal) {
141+
struct BuiltInAtomRequestSignal *request_signal
142+
= CONTAINER_OF(signal_message, struct BuiltInAtomRequestSignal, base);
143+
context_process_process_info_request_signal(ctx, request_signal);
144+
} else if (signal_message->type == MonitorSignal) {
145+
struct MonitorPointerSignal *monitor_signal
146+
= CONTAINER_OF(signal_message, struct MonitorPointerSignal, base);
147+
context_add_monitor(ctx, monitor_signal->monitor);
148+
} else if (signal_message->type == UnlinkSignal) {
149+
struct ImmediateSignal *immediate_signal
150+
= CONTAINER_OF(signal_message, struct ImmediateSignal, base);
151+
context_unlink(ctx, immediate_signal->immediate);
152+
} else if (signal_message->type == DemonitorSignal) {
153+
struct RefSignal *ref_signal
154+
= CONTAINER_OF(signal_message, struct RefSignal, base);
155+
context_demonitor(ctx, ref_signal->ref_ticks);
156+
}
157+
MailboxMessage *next = signal_message->next;
158+
mailbox_message_dispose(signal_message, &ctx->heap);
159+
signal_message = next;
160+
}
161+
136162
// When monitor message is sent, process is no longer in the table
137163
// and is no longer registered either.
138-
struct ResourceMonitor *resource_monitor = context_monitors_handle_terminate(ctx);
164+
struct Monitor *resource_monitors = context_monitors_handle_terminate(ctx);
139165

140166
synclist_unlock(&ctx->global->processes_table);
141167

142168
// Eventually call resource monitors handlers after the processes table was unlocked
143169
// The monitors were removed from the list of monitors.
144-
if (resource_monitor) {
170+
if (resource_monitors) {
145171
ErlNifEnv env;
146172
erl_nif_env_partial_init_from_globalcontext(&env, ctx->global);
147173

148174
struct ListHead monitors;
149-
list_prepend(&resource_monitor->base.monitor_list_head, &monitors);
175+
list_prepend(&resource_monitors->monitor_list_head, &monitors);
150176

151177
struct ListHead *item;
152178
struct ListHead *tmp;
153179
MUTABLE_LIST_FOR_EACH (item, tmp, &monitors) {
154180
struct Monitor *monitor = GET_LIST_ENTRY(item, struct Monitor, monitor_list_head);
155181
void *resource = term_to_term_ptr(monitor->monitor_obj);
156182
struct RefcBinary *refc = refc_binary_from_data(resource);
183+
resource_type_demonitor(refc->resource_type, monitor->ref_ticks);
157184
refc->resource_type->down(&env, resource, &ctx->process_id, &monitor->ref_ticks);
158185
free(monitor);
159186
}
@@ -201,18 +228,18 @@ void context_process_process_info_request_signal(Context *ctx, struct BuiltInAto
201228
if (context_get_process_info(ctx, NULL, &term_size, signal->atom, NULL)) {
202229
Heap heap;
203230
if (UNLIKELY(memory_init_heap(&heap, term_size) != MEMORY_GC_OK)) {
204-
mailbox_send_built_in_atom_signal(target, TrapExceptionSignal, OUT_OF_MEMORY_ATOM);
231+
mailbox_send_immediate_signal(target, TrapExceptionSignal, OUT_OF_MEMORY_ATOM);
205232
} else {
206233
term ret;
207234
if (context_get_process_info(ctx, &ret, NULL, signal->atom, &heap)) {
208235
mailbox_send_term_signal(target, TrapAnswerSignal, ret);
209236
} else {
210-
mailbox_send_built_in_atom_signal(target, TrapExceptionSignal, ret);
237+
mailbox_send_immediate_signal(target, TrapExceptionSignal, ret);
211238
}
212239
memory_destroy_heap(&heap, ctx->global);
213240
}
214241
} else {
215-
mailbox_send_built_in_atom_signal(target, TrapExceptionSignal, BADARG_ATOM);
242+
mailbox_send_immediate_signal(target, TrapExceptionSignal, BADARG_ATOM);
216243
}
217244
globalcontext_get_process_unlock(ctx->global, target);
218245
} // else: sender died
@@ -390,60 +417,53 @@ bool context_get_process_info(Context *ctx, term *out, size_t *term_size, term a
390417
return true;
391418
}
392419

393-
static struct ResourceMonitor *context_monitors_handle_terminate(Context *ctx)
420+
static struct Monitor *context_monitors_handle_terminate(Context *ctx)
394421
{
395422
GlobalContext *glb = ctx->global;
396423
struct ListHead *item;
397424
struct ListHead *tmp;
398-
struct ResourceMonitor *result = NULL;
425+
struct Monitor *result = NULL;
399426
MUTABLE_LIST_FOR_EACH (item, tmp, &ctx->monitors_head) {
400427
struct Monitor *monitor = GET_LIST_ENTRY(item, struct Monitor, monitor_list_head);
401-
if (monitor->ref_ticks && term_is_boxed(monitor->monitor_obj)) {
402-
// Resource monitor
403-
struct ResourceMonitor *resource_monitor = CONTAINER_OF(monitor, struct ResourceMonitor, base);
404-
// remove the monitor from the list of the resource
405-
list_remove(&resource_monitor->resource_list_head);
406-
list_init(&resource_monitor->resource_list_head);
428+
if (monitor->ref_ticks && ((monitor->monitor_obj & 0x3) == CONTEXT_MONITOR_RESOURCE_TAG)) {
407429
// remove it from the list we are iterating on
408430
if (result == NULL) {
409-
list_init(&resource_monitor->base.monitor_list_head);
410-
result = resource_monitor;
431+
list_init(&monitor->monitor_list_head);
432+
result = monitor;
411433
} else {
412-
list_append(&result->base.monitor_list_head, &resource_monitor->base.monitor_list_head);
434+
list_append(&result->monitor_list_head, &monitor->monitor_list_head);
413435
}
414-
} else {
436+
} else if (monitor->ref_ticks == 0 || ((monitor->monitor_obj & 0x3) == CONTEXT_MONITOR_MONITORED_PID_TAG)) {
437+
// term_to_local_process_id and monitor->monitor_obj >> 4 are identical here.
415438
int local_process_id = term_to_local_process_id(monitor->monitor_obj);
416439
Context *target = globalcontext_get_process_nolock(glb, local_process_id);
417440
if (IS_NULL_PTR(target)) {
418-
// TODO: we should scan for existing monitors when a context is destroyed
419-
// otherwise memory might be wasted for long living processes
441+
// TODO: assess whether this can happen
420442
free(monitor);
421443
continue;
422444
}
423445

424-
if (monitor->ref_ticks == 0 && (ctx->exit_reason != NORMAL_ATOM || target->trap_exit)) {
446+
if (monitor->ref_ticks == 0) {
447+
term exited_pid = term_from_local_process_id(ctx->process_id);
448+
mailbox_send_immediate_signal(target, UnlinkSignal, exited_pid);
425449
if (target->trap_exit) {
426450
if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(3)) != MEMORY_GC_OK)) {
427451
// TODO: handle out of memory here
428452
fprintf(stderr, "Cannot handle out of memory.\n");
429453
globalcontext_get_process_unlock(glb, target);
430454
AVM_ABORT();
431455
}
432-
433-
term exited_pid = term_from_local_process_id(ctx->process_id);
434-
// Process table should be locked before context_unlink is
435-
// called. This is done in calling function context_destroy.
436-
context_unlink(target, exited_pid);
437456
// Prepare the message on ctx's heap which will be freed afterwards.
438457
term info_tuple = term_alloc_tuple(3, &ctx->heap);
439458
term_put_tuple_element(info_tuple, 0, EXIT_ATOM);
440459
term_put_tuple_element(info_tuple, 1, exited_pid);
441460
term_put_tuple_element(info_tuple, 2, ctx->exit_reason);
442461
mailbox_send(target, info_tuple);
443-
} else {
462+
} else if (ctx->exit_reason != NORMAL_ATOM) {
444463
mailbox_send_term_signal(target, KillSignal, ctx->exit_reason);
445464
}
446-
} else if (monitor->ref_ticks) {
465+
} else {
466+
mailbox_send_ref_signal(target, DemonitorSignal, monitor->ref_ticks);
447467
int required_terms = REF_SIZE + TUPLE_SIZE(5);
448468
if (UNLIKELY(memory_ensure_free(ctx, required_terms) != MEMORY_GC_OK)) {
449469
// TODO: handle out of memory here
@@ -469,63 +489,71 @@ static struct ResourceMonitor *context_monitors_handle_terminate(Context *ctx)
469489
mailbox_send(target, info_tuple);
470490
}
471491
free(monitor);
492+
} else {
493+
// We are the monitoring process.
494+
int local_process_id = monitor->monitor_obj >> 4;
495+
Context *target = globalcontext_get_process_nolock(glb, local_process_id);
496+
mailbox_send_ref_signal(target, DemonitorSignal, monitor->ref_ticks);
497+
free(monitor);
472498
}
473499
}
474500
return result;
475501
}
476502

477-
int context_link(Context *ctx, term link_pid)
503+
struct Monitor *monitor_link_new(term link_pid)
478504
{
479-
struct ListHead *item;
480-
struct Monitor *monitor;
481-
LIST_FOR_EACH (item, &ctx->monitors_head) {
482-
monitor = GET_LIST_ENTRY(item, struct Monitor, monitor_list_head);
483-
if ((monitor->monitor_obj == link_pid) && (monitor->ref_ticks == 0)) {
484-
return 0;
485-
}
486-
}
487-
monitor = malloc(sizeof(struct Monitor));
505+
struct Monitor *monitor = malloc(sizeof(struct Monitor));
488506
if (IS_NULL_PTR(monitor)) {
489-
return -1;
507+
return NULL;
490508
}
491509
monitor->monitor_obj = link_pid;
492510
monitor->ref_ticks = 0;
493-
list_append(&ctx->monitors_head, &monitor->monitor_list_head);
494511

495-
return 0;
512+
return monitor;
496513
}
497514

498-
uint64_t context_monitor(Context *ctx, term monitor_pid)
515+
struct Monitor *monitor_new(term monitor_pid, uint64_t ref_ticks, bool is_monitoring)
499516
{
500-
uint64_t ref_ticks = globalcontext_get_ref_ticks(ctx->global);
501-
502517
struct Monitor *monitor = malloc(sizeof(struct Monitor));
503518
if (IS_NULL_PTR(monitor)) {
504-
return 0;
519+
return NULL;
520+
}
521+
int32_t local_process_id = term_to_local_process_id(monitor_pid);
522+
if (is_monitoring) {
523+
monitor->monitor_obj = (local_process_id << 4) | CONTEXT_MONITOR_MONITORING_PID_TAG;
524+
} else {
525+
monitor->monitor_obj = (local_process_id << 4) | CONTEXT_MONITOR_MONITORED_PID_TAG;
505526
}
506-
monitor->monitor_obj = monitor_pid;
507527
monitor->ref_ticks = ref_ticks;
508-
list_append(&ctx->monitors_head, &monitor->monitor_list_head);
509528

510-
return ref_ticks;
529+
return monitor;
511530
}
512531

513-
struct ResourceMonitor *context_resource_monitor(Context *ctx, void *resource)
532+
struct Monitor *monitor_resource_monitor_new(void *resource, uint64_t ref_ticks)
514533
{
515-
uint64_t ref_ticks = globalcontext_get_ref_ticks(ctx->global);
516-
517-
struct ResourceMonitor *monitor = malloc(sizeof(struct ResourceMonitor));
534+
struct Monitor *monitor = malloc(sizeof(struct Monitor));
518535
if (IS_NULL_PTR(monitor)) {
519536
return NULL;
520537
}
521-
// Not really boxed, but sufficient to distinguish from pids
522-
monitor->base.monitor_obj = ((term) resource) | TERM_BOXED_VALUE_TAG;
523-
monitor->base.ref_ticks = ref_ticks;
524-
list_append(&ctx->monitors_head, &monitor->base.monitor_list_head);
538+
monitor->monitor_obj = ((term) resource) | CONTEXT_MONITOR_RESOURCE_TAG;
539+
monitor->ref_ticks = ref_ticks;
525540

526541
return monitor;
527542
}
528543

544+
void context_add_monitor(Context *ctx, struct Monitor *new_monitor)
545+
{
546+
struct ListHead *item;
547+
LIST_FOR_EACH (item, &ctx->monitors_head) {
548+
struct Monitor *existing = GET_LIST_ENTRY(item, struct Monitor, monitor_list_head);
549+
if ((existing->monitor_obj == new_monitor->monitor_obj) && (existing->ref_ticks == new_monitor->ref_ticks)) {
550+
free(new_monitor);
551+
return;
552+
}
553+
}
554+
list_append(&ctx->monitors_head, &new_monitor->monitor_list_head);
555+
}
556+
529557
void context_unlink(Context *ctx, term link_pid)
530558
{
531559
struct ListHead *item;
@@ -538,3 +566,36 @@ void context_unlink(Context *ctx, term link_pid)
538566
}
539567
}
540568
}
569+
570+
void context_demonitor(Context *ctx, uint64_t ref_ticks)
571+
{
572+
struct ListHead *item;
573+
LIST_FOR_EACH (item, &ctx->monitors_head) {
574+
struct Monitor *monitor = GET_LIST_ENTRY(item, struct Monitor, monitor_list_head);
575+
if (monitor->ref_ticks == ref_ticks) {
576+
list_remove(&monitor->monitor_list_head);
577+
free(monitor);
578+
return;
579+
}
580+
}
581+
}
582+
583+
term context_get_monitor_pid(Context *ctx, uint64_t ref_ticks, bool *is_monitoring)
584+
{
585+
struct ListHead *item;
586+
LIST_FOR_EACH (item, &ctx->monitors_head) {
587+
struct Monitor *monitor = GET_LIST_ENTRY(item, struct Monitor, monitor_list_head);
588+
if (monitor->ref_ticks == ref_ticks) {
589+
if ((monitor->monitor_obj & 0x3) == CONTEXT_MONITOR_MONITORED_PID_TAG) {
590+
*is_monitoring = false;
591+
return term_from_local_process_id(monitor->monitor_obj >> 4);
592+
} else if ((monitor->monitor_obj & 0x3) == CONTEXT_MONITOR_MONITORING_PID_TAG) {
593+
*is_monitoring = true;
594+
return term_from_local_process_id(monitor->monitor_obj >> 4);
595+
} else {
596+
return term_invalid_term();
597+
}
598+
}
599+
}
600+
return term_invalid_term();
601+
}

0 commit comments

Comments
 (0)