When removing a hash entry that was in the cell assigned to its hash, we
would clear the first bit in the second maps. If this entry was a
secondary value with the same hash, then this value became unreachable.
This very rarely showed up for two reasons. First, most of the tables
are insert-only and so we never try to remove things from them. Second,
it requires a particular sequence of inserts. It occasionally caused
weak references to be susceptible to use after free.
This commit fixes a data loss bug in our hopscotch table implementation.
Removing values from the table can result in other values becoming
disconnected and lost.
Let A, B, and C be values that all hash to cell 0.
Assume the hopscotch distance factor H = 2.
0 1 2
+-----+-----+-----+
| | | |
+-----+-----+-----+
After adding A
0 1 2
+-----+-----+-----+
| A | | |
+-----+-----+-----+
|
+-Neighbors =
After adding B
0 1 2
+-----+-----+-----+
| A | B | |
+-----+-----+-----+
|
+-Neighbors = 1
After adding C
0 1 2
+-----+-----+-----+
| A | B | C |
+-----+-----+-----+
|
+-Neighbors = 1, 2
If we then remove B,
0 1 2
+-----+-----+-----+
| A | [X] | C |
+-----+-----+-----+
|
+-Neighbors = 1, 2
* It is replaced with a placeholder [X].
* A's neighbor table is not updated to reflect the loss.
If we then remove A,
0 1 2
+-----+-----+-----+
| [X] | [X] | [C] |
+-----+-----+-----+
|
+-Neighbors = 2
* The table is rebalanced to promote A's lowest neighbor to the primary
cell position.
* C from cell 2 remains cell 0's neighbor.
The bug manifests if [X] the placeholder value passes the null check set
out in MAP_TABLE_VALUE_NULL; that is, the placeholder is "effectively
null".
Looking up the key that matches C will first evaluate its base cell, the
one that collided with the key in the first place. Since that is
placeholder [X], and [X] is "effectively null", the lookup stops.
C is never retrieved from the hash table.
---
The expedient solution to this bug is to update cell 0's neighbors when
B is first removed, effectively skipping the hole:
If we remove B as above,
0 1 2
+-----+-----+-----+
| A | [X] | C |
+-----+-----+-----+
|
+-Neighbors = 2 <<< HERE
but clear the neighbor bit for cell 1, the promotion that happens when A
is later removed places C in cell 0.
0 1 2
+-----+-----+-----+
| C | [X] | [X] |
+-----+-----+-----+
|
+-Neighbors =
recover memory from the smaller ones. Selector lookups are not on the critical
path for message sends, so the cost of the extra memory is likely to be larger
than the cost of acquiring a lock in sel_getName().
Store the refcount structures inside the hash table, rather than in a chained structure. This uses less space and should be easier for the GC to scan (less cache used).
Imported selector table code frm the Étoilé runtime. We can now make dispatch type dependent with a -D switch. Not enabled yet, but it will be enabled in a warning mode soon - I consider preferable to the existing GNU and Apple solution of corrupting the stack.
Although GCKit is mostly finished, it is still not well tested. It contains bugs, and possibly dragons. Do not use it. If you disregard this advice, do not file any bug reports. If you disregard this instruction, then I will point and laugh at you.
The protocols table and the class table will now grow as required. Protocols are now added to the protocols table as they are created, so you should get something sensible and introspectable back when you look up a protocol by name.
Note that concurrent resizing has not yet been implemented. That means that there is a hard limit on the number of classes that can be loaded. This is currently set to quite a small number for testing, to stress the hash table. If you're experiencing problems as a result, then please let me know and I will increase it.
David Chisnall
Dustin L. Howett
David Chisnall
theraven
rfm