@polarathene would providing the one failing test with more time help here, or do you think the issue is somewhere else entirely?

This is probably related to PR #2157.

This is probably related to PR #2157.

It is :D But #2157 has stalled.

I'm not even sure whether this PR is the right way, but I'm running my deployment with 10s and all is well for me. Independently of the problem I think that increasing sleep time is a good idea.

Fixes nothing, but should increase the time before DMS starts using all the RAM on a system.

I think a better approach would be, to restart the changedetector service once a day or so.
Your solution does only slow down a potential memory leak, but will not prevent it. Restarting the service should however "fix" it.

Good idea. Will supervisorctl changedetector restart suffice here?

Yes, I think so. But haven't tested it. If the PID of the script changes afterwards, it works.

BTW: While reviewing the check-for-changes script, I stumbled upon

@georglauterbach @polarathene Do you have an idea, why there is a subshell used + exec statement? IMO it should work without those too. I cannot imagine a reason for that.

BTW: While reviewing the check-for-changes script, I stumbled upon

docker-mailserver/target/scripts/helper-functions.sh

Lines 245 to 254 in 7b21db7

	(
	cd /tmp/docker-mailserver || exit 1
	exec sha512sum 2>/dev/null -- \
	postfix-accounts.cf \
	postfix-virtual.cf \
	postfix-aliases.cf \
	dovecot-quotas.cf \
	/etc/letsencrypt/acme.json \
	${CERT_FILES[@]}
	)

@georglauterbach @polarathene Do you have an idea, why there is a subshell used + exec statement? IMO it should work without those too. I cannot imagine a reason for that.

I see no reason why the subshell is there either... maybe the writer wanted to be able to not run the SHA-sum when he/she could not change directories, but this is an awful way of doing it. I think this can be done in a better way :D

I think this can be done in a better way :D

TBH, this spot in the code is the strange part that makes me feel it's the culprit. I wish that, if you folks could refactor this to something less astonishing, we could then test whether that actually removes the issue, before having a PR applied that could daily kill the problem.

I think this can be done in a better way :D

TBH, this spot in the code is the strange part that makes me feel it's the culprit. I wish that, if you folks could refactor this to something less astonishing, we could then test whether that actually removes the issue, before having a PR applied that could daily kill the problem.

I will see where I can provide something here unless @casperklein beats me to it :D @casperklein maybe you can have a look at the mailbox in #2361 if you have some time - not sure how to approach this properly, and you have more experience in this regard.

function _fix_restart_changedetector_daily

One last thing: I think it's a good idea, to add a small comment referencing the issue, so it's clear, why this function exists.
Otherwise it LGTM 👍

Edit:

before having a PR applied that could daily kill the problem.

Valid point. Once this "fix" is applied and works, people will not notice that there is a problem, which makes it harder to find the root cause and apply a final fix.

Just to be clear, I suspect of sub shell part because it does a full clone of shell script state (without due reason afaik), and this state cloning might be increasing each time it happens, accumulating over previous state (although I couldn't yet realize what could be linearly increasing), while leaving leftovers behind in the parent shell process. This can explain why both RAM and CPU usage increases linearly.

Just to be clear, I suspect of sub shell part because it does a full clone of shell script state (without due reason afaik), and this state cloning might be increasing each time it happens, accumulating over previous state (although I couldn't yet realize what could be linearly increasing), while leaving leftovers behind in the parent shell process. This can explain why both RAM and CPU usage increases linearly.

I think that's a really good point.

I've seen exec used instead of eval but both for the same purpose: allowing interpolation of dynamic variables to happen before execution. Likely why because of the array expansion/interpolation happening. Though, it should work just fine without exec afaik

Currently testing the new version locally. If this works, I'll create another PR and we can close this one.

While the removal of exec is certainly worthwhile, if this fixes the resource leak, it seems as if the problem is with the version of Bash on Debian 11 (some bug in Bash?), or the exec does something very weird we do not understand. I highly doubt that this is a problem with the underlying exec syscall, but maybe Docker does a weird translation as well. Who knows...

I will close this in favor of #2401.

Just to add to the discussion. It was maybe related to syscall change with the newer Debian image, perhaps not due to Bash itself but another core package (EDIT: glibc AFAIK).

I don't recall the specifics but Alpine has a similar problem I became aware of a week or so ago (EDIT: this answer explains it, for Debian 10 vs 11 it seems to be related to the glibc update, notably with kernels prior to 5.8), where a change since their 3.14 release resulted in a syscall that older kernels didn't support properly.. I was surprised as it broke the hash command from bash from working correctly (the Sentry.io CLI installer script was failing because of that).

Very interesting, so it was as I suspected earlier in the issue discussion (a kernel thing...). Thank you for sharing the information here, very much appreciated!

EDIT: glibc is broken anyway (if one belives L. Torvalds :D)

@polarathene tbh I'm not sure how that relates to a leak due to subshell'ing, I couldn't connect the dots. Pointed problem refers to failing testing file writable, seems not related at first.