| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
| |
None of these are harmful, and they are almost certainly optimized
away by the compiler. The motivation for fixing them anyway is that
we'd like to enable static analysis as part of CI, and the first step
towards that is resolving the warnings it produces at present.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Static analysis flagged this issue. Here is a minimal program which
causes a segfault within Jemalloc:
```
#include <jemalloc/jemalloc.h>
const char *malloc_conf = "prof:true";
int main() {
mallctl("prof.prefix", NULL, NULL, NULL, 0);
}
```
Fixed by checking if `prefix` is `NULL`.
|
|
|
|
|
|
| |
As pointed out in #2434, the thread_name in prof_tdata_t was changed in #2407.
This also requires an update for the prof_recent dump, specifically the emitter
expects a "char **" which is fixed in this commit.
|
|
|
|
| |
and use it on the background thread name setting.
|
| |
|
| |
|
|
|
|
|
| |
Static analysis flagged this. Fixed by simply checking `oldlenp`
before dereferencing it.
|
|
|
|
|
|
|
|
| |
Static analysis flagged this. `extent_record` was passing `NULL` as the
value for `coalesced` to `extent_try_coalesce`, which in turn passes
that argument to `extent_try_coalesce_impl`, where it is written to
without checking if it is `NULL`. I can confirm from reviewing the
fleetwide coredump data that this was in fact being hit in production.
|
|
|
|
|
|
| |
The codebase is already very disciplined in making any function which
can be `static`, but there are a few that appear to have slipped through
the cracks.
|
|
|
|
|
|
|
| |
This codepath may generate deferred work when the HPA is enabled.
See also [@davidtgoldblatt's relevant comment on the PR which
introduced this](https://github.com/jemalloc/jemalloc/pull/2107#discussion_r699770967)
which prevented a similarly incorrect `assert` from being added elsewhere.
|
|
|
|
|
|
|
|
|
|
| |
It appears like a simple typo means we're unconditionally overwriting
some fields in hpa_from_pai when asserts are enabled. From hpa_shard_init,
it looks like these fields have these values anyway, so this shouldn't
cause bugs, but if something is wrong it seems better to have these
asserts in place.
See issue #2412.
|
|
|
|
|
|
|
|
|
| |
The previous approach managed the thread name in a separate buffer, which causes
races because the thread name update (triggered by new samples) can happen at
the same time as prof dumping (which reads the thread names) -- these two
operations are under separate locks to avoid blocking each other. Implemented
the thread name storage as part of the tdata struct, which resolves the lifetime
issue and also avoids internal alloc / dalloc during prof_sample.
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
| |
The current thread name reading path updates the name every time, which requires
both alloc and dalloc -- and the temporary NULL value in the middle causes races
where the prof dump read path gets NULLed in the middle.
Minimize the changes in this commit to isolate the bugfix testing; will also
refactor the whole thread name paths later.
|
|
|
|
|
|
|
|
|
|
|
| |
The added hooks hooks.prof_sample and hooks.prof_sample_free are intended to
allow advanced users to track additional information, to enable new ways of
profiling on top of the jemalloc heap profile and sample features.
The sample hook is invoked after the allocation and backtracing, and forwards
the both the allocation and backtrace to the user hook; the sample_free hook
happens before the actual deallocation, and forwards only the ptr and usz to the
hook.
|
| |
|
|
|
|
|
|
| |
Allows the use of getenv() rather than secure_getenv() to read MALLOC_CONF.
This helps in situations where hosts are under full control, and setting
MALLOC_CONF is needed while also setuid. Disabled by default.
|
|
|
|
|
|
|
|
|
|
| |
Previously if a thread does only allocations, it stays on the slow path /
minimal initialized state forever. However, dealloc-only is a valid pattern for
dedicated reclamation threads -- this means thread cache is disabled (no batched
flush) for them, which causes high overhead and contention.
Added the condition to fully initialize TSD when a fair amount of dealloc
activities are observed.
|
| |
|
|
|
|
| |
An arena-level name can help identify manual arenas.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
invalid in C99
./autogen.sh \
&& ./configure --prefix=/usr/local --enable-static --enable-autogen --enable-xmalloc --with-static-libunwind=/usr/local/lib/libunwind.a --enable-lazy-lock --with-jemalloc-prefix='' \
&& make -j16
...
gcc -std=gnu11 -Werror=unknown-warning-option -Wall -Wextra -Wshorten-64-to-32 -Wsign-compare -Wundef -Wno-format-zero-length -Wpointer-arith -Wno-missing-braces -Wno-missing-field-initializers -pipe -g3 -Wimplicit-fallthrough -O3 -funroll-loops -fPIC -DPIC -c -D_REENTRANT -Iinclude -Iinclude -DJEMALLOC_NO_PRIVATE_NAMESPACE -o src/edata_cache.sym.o src/edata_cache.c
src/background_thread.c:768:6: error: implicit declaration of function 'pthread_create_fptr_init' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
pthread_create_fptr_init()) {
^
src/background_thread.c:768:6: note: did you mean 'pthread_create_wrapper_init'?
src/background_thread.c:34:1: note: 'pthread_create_wrapper_init' declared here
pthread_create_wrapper_init(void) {
^
1 error generated.
make: *** [src/background_thread.sym.o] Error 1
make: *** Waiting for unfinished jobs....
|
|
|
|
| |
Refactored cache_bin.h so that only one function is racy.
|
|
|
|
|
|
| |
Add new runtime option `debug_double_free_max_scan` that specifies the max
number of stack entries to scan in the cache bit when trying to detect the
double free bug (currently debug build only).
|
|
|
|
|
| |
- enabling pthread_get/pthread_set_name_np api.
- disabling per thread cpu affinity handling, unsupported on this platform.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In arena_stats_merge() first nmalloc was read, and after ndalloc.
However with this order, it is possible for some thread to incement
ndalloc in between, and then nmalloc < ndalloc, and assertion will fail,
like again found by ClickHouse CI [1] (even after #2234).
[1]: https://github.com/ClickHouse/ClickHouse/issues/31531
Swap the order to avoid possible assertion.
Cc: @interwq
Follow-up for: #2234
|
|
|
|
|
|
|
| |
The option makes jemalloc use prctl with PR_SET_VMA to tag memory mappings with
"jemalloc_pg" or "jemalloc_pg_overcommit". This allows to easily identify
jemalloc's mappings in /proc/<pid>/maps. PR_SET_VMA is only available in Linux
5.17 and above.
|
|
|
|
|
| |
Despite being an obsolete function, pvalloc is still present in GLIBC and should
work correctly when jemalloc replaces libc allocator.
|
| |
|
| |
|
|
|
|
|
| |
Allow setting the safety check abort hook through mallctl, which avoids abort()
and core dumps.
|
|
|
|
| |
Signed-off-by: cuishuang <imcusg@gmail.com>
|
|
|
|
|
| |
With realloc(ptr, 0) being UB per C23, the option name "strict" makes less sense
now. Rename to "alloc" which describes the behavior.
|
|
|
|
|
|
| |
Default SEC max_alloc option value was 32k, disabling SEC for platforms with
lg-page=16. This change enables SEC for all platforms, making minimum max_alloc
value equal to PAGE.
|
| |
|
| |
|
|
|
|
|
|
| |
Due to a bug in sec initialization, the number of cached size classes
was equal to 198. The bug caused the creation of more than a hundred of
unused bins, although it didn't affect the caching logic.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before this commit, in case FreeBSD libc jemalloc was overridden by another
jemalloc, proper thread shutdown callback was involved only for the overriding
jemalloc. A call to _malloc_thread_cleanup from libthr would be redirected to
user jemalloc, leaving data about dead threads hanging in system jemalloc. This
change tackles the issue in two ways. First, for current and old system
jemallocs, which we can not modify, the overriding jemalloc would locate and
invoke system cleanup routine. For upcoming jemalloc integrations, the cleanup
registering function will also be redirected to user jemalloc, which means that
system jemalloc's cleanup routine will be registered in user's jemalloc and a
single call to _malloc_thread_cleanup will be sufficient to invoke both
callbacks.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
It is possible that ndalloc will be updated before nmalloc, in
arena_large_ralloc_stats_update(), fix this by reorder those calls.
It was found by ClickHouse CI, that periodically hits this assertion [1].
[1]: https://github.com/ClickHouse/ClickHouse/issues/31531
That issue contains lots of examples, with core dump and some gdb output [2].
[2]: https://s3.amazonaws.com/clickhouse-test-reports/34951/96390a9263cb5af3d6e42a84988239c9ae87ce32/stress_test__debug__actions_.html
Here you can find binaries for that particular report [3] you need
clickhouse debug build [4].
[3]: https://s3.amazonaws.com/clickhouse-builds/34951/96390a9263cb5af3d6e42a84988239c9ae87ce32/clickhouse_build_check_(actions)/report.html
[4]: https://s3.amazonaws.com/clickhouse-builds/34951/96390a9263cb5af3d6e42a84988239c9ae87ce32/package_debug/clickhouse
Brief info from that report:
2 0x000000002ad6dbfe in arena_stats_merge (tsdn=0x7f2399abdd20, arena=0x7f241ce01080, nthreads=0x7f24e4360958, dss=0x7f24e4360960, dirty_decay_ms=0x7f24e4360968, muzzy_decay_ms=0x7f24e4360970, nactive=0x7f24e4360978, ndirty=0x7f24e43
e4360988, astats=0x7f24e4360998, bstats=0x7f24e4363310, lstats=0x7f24e4364990, estats=0x7f24e4366e50, hpastats=0x7f24e43693a0, secstats=0x7f24e436a020) at ../contrib/jemalloc/src/arena.c:138
ndalloc = 226
nflush = 0
curlextents = 0
nmalloc = 225
nrequests = 0
Here you can see that they differs only by 1.
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
|
|
|
|
|
|
|
|
|
|
| |
While calculating the number of stashed pointers, multiple variables
potentially modified by a concurrent thread were used for the
calculation. This led to some inconsistencies, correctly detected by
the assertions. The change eliminates some possible inconsistencies by
using unmodified variables and only once a concurrently modified one.
The assertions are omitted for the cases where we acknowledge potential
inconsistencies too.
|
|
|
|
| |
Currently only prof_leak_error and prof_final are checked.
|
|
|
|
|
| |
Otherwise, prof_leak may get set after prof_leak_error, and disagree with each
other.
|
|
|
|
|
|
| |
The option makes the process to exit with error code 1 if a memory leak
is detected. This is useful for implementing automated tools that rely
on leak detection.
|
| |
|
|
|
|
|
| |
This makes debugging slightly easier and avoids the confusion of "should we
create new arenas" here.
|
|
|
|
|
|
| |
With DSS as primary, the default merge impl will (correctly) decline to merge
when one of the extent is non-dss. The error path should tolerate the
not-merged extent being in a merging state.
|
| |
|