Fix a Windows crash on exit without 'quit' command #2679
Conversation
There was a bug in commit d476342 (Add support for Windows large pages) that could result in trying to free memory allocated with VirtualAlloc incorrectly with free(). Fix this by reverting the TT.resize(0) logic in the previous commit, and instead, we'll just call aligned_ttmem_free() in TranspositionTable::~TranspositionTable().
|
Sorry about this. With this patch amended, this is how the original patch was supposed to be. |
| @@ -75,7 +75,7 @@ class TranspositionTable { | |||
| static_assert(sizeof(Cluster) == 32, "Unexpected Cluster size"); | |||
|
|
|||
| public: | |||
| ~TranspositionTable() { free(mem); } | |||
|
thanks for the quick analysis, looks like a clean fix. I'll merge later tonight, if there are no more remarks. |
|
Created a compile with this fix on top. Looks good now, No crashes or errors showing up in Windows log. The 2 to 3 second hang is also gone and closing is now instantaneous (ie normal behavior) for the two methods of closing that were lagging before: Ctrl-C and "X". The only remaining (minor) thing is that on typing command "bench", the phrase "info string Hash table allocation: Windows large pages not used." is printed out twice, in addition to it initially printing out once when the executable is first started, so a total of three times. This also seems to happen with the initial Large Pages commit so it isn't something new introduced by the fix. Not sure if this is expected behavior or not but either way it doesn't seem to impact the functionality. So, looks good to me too, thanks :) |
|
The printing is more or less expected behavior, just reflects how often we allocate. Shouldn't be changed in the current PR. |
|
Thanks for fixing things quickly! |
There was a bug in commit d476342
(Add support for Windows large pages) that could result in trying to
free memory allocated with VirtualAlloc incorrectly with free().
Fix this by reverting the TT.resize(0) logic in the previous commit,
and instead, we'll just call aligned_ttmem_free() in
TranspositionTable::~TranspositionTable().