gh-143807: Limit executor growth#143814
Conversation
|
Hmmm, can we confirm that this fixes the assertion failure on @devdanzin's system then at least, since this could at least be repro'd there? |
|
I'll be able to test in about 6 hours. |
|
Sorry about the delay, had to bisect it not reproducing on HEAD. e370c8d has made it stop aborting: Does that make sense? Applying the patch (edit: and running the MRE) leads to a segfault (with or without ASan), can you reproduce this? ASan output: Backtrace: Output of running with |
|
Thanks @devdanzin. The backtrace looks like the original repro for that other issue that you said fixed it thougj, so thats strange In any case, this is an actual logic bug we should fix. So even if it doesnt manifest for us we should fix it. |
| } | ||
| assert(size <= capacity); | ||
| if (size == capacity) { | ||
| if (capacity * 2 >= MAX_EXECUTORS_SIZE) { |
There was a problem hiding this comment.
This would be easier to follow if we did the test on new_capacity below:
if (new_capacity >= MAX_EXECUTORS_SIZE) {
return -1;
No unit test, because the repro provided doesn't repro on my system.
The requirement is there can only be 255 executors in a single code object, as the executor index is bounded by
oparg, whch is aint8_t.size < MAX_EXECUTORS_SIZEfailed inget_index_for_executor#143807