Skip to content

Commit c40e949

Browse files
committed
Revert "Fix mpmcq use-after-free bug (#4541)"
This reverts commit b5decba. The fix of a theoretical use after free (which is currently not actually an issue) is causing a very real world explosion in memory usage for some programs.
1 parent 0eca83c commit c40e949

File tree

3 files changed

+2
-37
lines changed

3 files changed

+2
-37
lines changed

src/libponyrt/sched/mpmcq.c

Lines changed: 2 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#define PONY_WANT_ATOMIC_DEFS
22

33
#include "mpmcq.h"
4-
#include "ponyassert.h"
54
#include "../mem/pool.h"
65
#include "../sched/cpu.h"
76

@@ -17,19 +16,9 @@ struct mpmcq_node_t
1716
PONY_ATOMIC(void*) data;
1817
};
1918

20-
static __pony_thread_local mpmcq_node_t* nodes_to_recycle;
21-
2219
static mpmcq_node_t* node_alloc(void* data)
2320
{
24-
mpmcq_node_t* node = nodes_to_recycle;
25-
26-
if (NULL != node)
27-
{
28-
nodes_to_recycle = atomic_load_explicit(&node->next, memory_order_relaxed);
29-
} else {
30-
node = POOL_ALLOC(mpmcq_node_t);
31-
}
32-
21+
mpmcq_node_t* node = POOL_ALLOC(mpmcq_node_t);
3322
atomic_store_explicit(&node->next, NULL, memory_order_relaxed);
3423
atomic_store_explicit(&node->data, data, memory_order_relaxed);
3524

@@ -66,7 +55,6 @@ static size_t mpmcq_size_debug(mpmcq_t* q)
6655

6756
void ponyint_mpmcq_init(mpmcq_t* q)
6857
{
69-
nodes_to_recycle = NULL;
7058
mpmcq_node_t* node = node_alloc(NULL);
7159

7260
atomic_store_explicit(&q->head, node, memory_order_relaxed);
@@ -78,20 +66,8 @@ void ponyint_mpmcq_init(mpmcq_t* q)
7866
#endif
7967
}
8068

81-
void ponyint_mpmcq_cleanup()
82-
{
83-
while(nodes_to_recycle != NULL)
84-
{
85-
mpmcq_node_t* node = nodes_to_recycle;
86-
nodes_to_recycle = atomic_load_explicit(&node->next, memory_order_relaxed);
87-
node_free(node);
88-
}
89-
}
90-
9169
void ponyint_mpmcq_destroy(mpmcq_t* q)
9270
{
93-
pony_assert(atomic_load_explicit(&q->head, memory_order_relaxed) == q->tail.object);
94-
9571
atomic_store_explicit(&q->head, NULL, memory_order_relaxed);
9672
node_free(q->tail.object);
9773
q->tail.object = NULL;
@@ -201,14 +177,7 @@ void* ponyint_mpmcq_pop(mpmcq_t* q)
201177
ANNOTATE_HAPPENS_AFTER(&tail->data);
202178
#endif
203179

204-
// if tail has already been consumed and freed by this thread, the read of
205-
// `tail->next` above in another thread would be a use-after-free error but is
206-
// technically safe in this context since we will end up looping due to the CAS
207-
// failing in the while condition due to the aba counter being out of sync
208-
// we cannot free the tail node if we want to ensure there are no use-after-free
209-
// concerns so we instead add it to the list of nodes to recycle
210-
atomic_store_explicit(&tail->next, nodes_to_recycle, memory_order_relaxed);
211-
nodes_to_recycle = tail;
180+
node_free(tail);
212181

213182
return data;
214183
}

src/libponyrt/sched/mpmcq.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ void ponyint_mpmcq_init(mpmcq_t* q);
2424

2525
void ponyint_mpmcq_destroy(mpmcq_t* q);
2626

27-
void ponyint_mpmcq_cleanup();
28-
2927
void ponyint_mpmcq_push(mpmcq_t* q, void* data);
3028

3129
void ponyint_mpmcq_push_single(mpmcq_t* q, void* data);

src/libponyrt/sched/scheduler.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1305,8 +1305,6 @@ static DECLARE_THREAD_FN(run_thread)
13051305
#endif
13061306

13071307
run(sched);
1308-
1309-
ponyint_mpmcq_cleanup();
13101308
ponyint_pool_thread_cleanup();
13111309

13121310
return 0;

0 commit comments

Comments
 (0)