search

ivmai / bdwgc

The Boehm-Demers-Weiser conservative C/C++ Garbage Collector (bdwgc, also known as bdw-gc, boehm-gc, libgc)
https://www.hboehm.info/gc/
Other
2.98k stars 407 forks source link

gctest fails infrequently (Unexpected heap growth) with Mingw 32-bit #264

Closed Hamayama closed 5 years ago

Hamayama commented 5 years ago

I made a local branch (mingw32test) from master and tested on AppVeyor. https://github.com/Hamayama/bdwgc/commit/199f150a26ca24d1871555ac808e1d542aa63c76 https://ci.appveyor.com/project/Hamayama/bdwgc/builds/22297190

gctest fails infrequently (Unexpected heap growth) with Mingw 32-bit. https://ci.appveyor.com/project/Hamayama/bdwgc/builds/22297190#L508

I tried 'git bisect start 9483d5b c7837f8' and found the following commit.

It seems to allocate heap memory though I don't know the size ...

ivmai commented 5 years ago

Hmm. The patch is small and looks innocent. Please trying commenting various lines of the patch, e.g. GC_FREE(GC_MALLOC_ATOMIC_IGNORE_OFF_PAGE(1))

Hamayama commented 5 years ago

I made branches from mingw32test and tested on AppVeyor.

There seems to be other causes ...

Hamayama commented 5 years ago

I can't reproduce this issue on my pc (Windows 8.1 (64bit), MSYS2/MinGW-w64 (32bit)). I can reproduce this issue on AppVeyor only.

Next, I will try to collect heap size data (e.g. min/max/mean) of gctest for some branches.

I think this issue is not a serious problem. So, I will do it at my own pace ...

ivmai commented 5 years ago

So, I will do it at my own pace ...

Cool! Thank you

ivmai commented 5 years ago

I think it is not a bug, just the tested condition should be relaxed slightly (of course, it would be good to understand why the condition is violated)

Hamayama commented 5 years ago

I found that GC_disable(); in run_one_test in tests/test.c seems to affect all threads.

I commented out this and confirmed that total heap size decreases.

before: https://github.com/Hamayama/bdwgc/commit/aedc5df480ef7ffaae11e3eb3238d57942873c93

after: https://github.com/Hamayama/bdwgc/commit/a7b2fd502013a4ddd2b6ebe7a3ddde0771f4fb73

result: https://drive.google.com/open?id=1H9HePh6yhJl62OnfJJ0f5wZK--is-Re4gqhHZ1v5hOg ( This is google spreadsheet. See AV32 and AV32 Fix sheets. )

ivmai commented 5 years ago

I see the root cause - in the multi-threaded case, one thread may do GC_disable and be suspended by OS scheduler (especially you can see it on a single or dual-core CPU like AppVeyor) while other threads may want to allocate a lot of memory. I'll make the fix - move GC_disable/enable testing to some other place when only a single client thread exists.

Good catch, thank you. Sometimes an innocent code turns out to be guilty.

ivmai commented 5 years ago

I think the issue is fixed.

Hamayama commented 5 years ago

I tested master and OK. Thanks! https://drive.google.com/open?id=1H9HePh6yhJl62OnfJJ0f5wZK--is-Re4gqhHZ1v5hOg (AV32 Update sheet)