search

foxcpp / maddy

✉️ Composable all-in-one mail server.
https://maddy.email
GNU General Public License v3.0
5.13k stars 249 forks source link

race condition on table.file #557

Closed domcyrus closed 10 months ago

domcyrus commented 1 year ago

Describe the bug

There is a race condition in table.file module. The problem is related to the comment on https://github.com/foxcpp/maddy/blob/master/internal/table/file_test.go#L114

    // This delay is somehow important. Not sure why.
    time.Sleep(500 * time.Millisecond)

Steps to reproduce

go test -run TestFileReload -count 1000

Eventually you will hit the issue and the unit test will fail with:

--- FAIL: TestFileReload (3.51s)
    file_test.go:134: New m were not loaded

The problem is that in unix/linux OS the code which creates the file is not atomic and therefore there is a race condition that although the code is stat'ing the file it will actually read an empty file instead of the content cat: dog.

Now the question is how to fix this. It is trivial to fix the unit test in a way that we use the standard procedure to first write a tempfile with the content and then do an atomic rename of the file. This would fix the unit test BUT I think we should actually also fix the real issue in the code e.g. in that it should not only check the last modified date but also whether content is already written to the file.

domcyrus commented 1 year ago

After the discussion on IRC we will skipping changes if the last time since modified is less than the reload interval. FYI @foxcpp