Skip to content

Commit 3c6eeda

Browse files
committed
Array.sort: fix issue where *some* sorts of 10+ items could cause the array not to be GC'd
1 parent af141a9 commit 3c6eeda

File tree

2 files changed

+13
-3
lines changed

2 files changed

+13
-3
lines changed

ChangeLog

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
:
1+
: Array.sort: fix issue where *some* sorts of 10+ items could cause the array not to be GC'd
22

33
2v28 : Add `E.internal` as a way to access the 'hidden root' containing Espruino internal variables that previously needed `global["\xff"]`
44
Bangle.js: Fix back handler not removed when using E.setUI with a back button but without widgets (#2636)

src/jswrap_array.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -823,9 +823,11 @@ NO_INLINE static void _jswrap_array_sort(JsvIterator *head, int n, JsVar *compar
823823
JsvIterator pivot;
824824
jsvIteratorClone(&pivot, head);
825825
#ifndef SAVE_ON_FLASH
826-
if (n>16) {
826+
if (n>10) {
827827
/* if we have enough elements where using the wrong pivot may be a problem, try a median of 3
828-
to find the best pivot - this can be slower as we have to iterate over all the items in the list */
828+
to find the best pivot - this can be slower as we have to iterate over all the items in the list.
829+
16 appears to be the most efficient for # of compares for sorted arrays, but 10 allows us to
830+
lower the amount of recursion we do. */
829831
int midIndex = n>>1;
830832
JsvIterator mid;
831833
jsvIteratorClone(&mid, head);
@@ -968,9 +970,17 @@ JsVar *jswrap_array_sort (JsVar *array, JsVar *compareFn) {
968970
n = (int)jsvGetLength(array);
969971
}
970972

973+
unsigned char locks = jsvGetLocks(array);
971974
jsvIteratorNew(&it, array, JSIF_EVERY_ARRAY_ELEMENT);
972975
_jswrap_array_sort(&it, n, compareFn);
973976
jsvIteratorFree(&it);
977+
/* This is really nasty, but sometimes the amount of recursion the quicksort does on
978+
an array might push us past our maximum amount of locks, in which case it
979+
stays at a max of JSV_LOCK_MAX and can't be freed even with GC. In this case
980+
we detect this and just set the lock count back to what it was before. */
981+
if (jsvGetLocks(array) == JSV_LOCK_MAX)
982+
array->flags = (array->flags&~JSV_LOCK_MASK) | (locks<<JSV_LOCK_SHIFT);
983+
974984
return jsvLockAgain(array);
975985
}
976986

0 commit comments

Comments
 (0)