#6502 Race condition when loading or updating project files Issue created by pandemicsyn The `ProjectFile` implementation as is today isn't thread safe and has a potential problematic race condition. This isn't really an issue yet, but will likely become one if we work on API v2 or possibly if something

melturbo

like ever comes to fruition. This manifested for the first time (afaik) in #6297. While our Uvicorn implementation is a WIP, I think this issue pop'd up mostly as a side effect of uvicorn allowing more concurrency and exposing the underlying issue. The easiest way to trigger this was by making concurrent call's to

/api/v1/plugins/all

, which then typically triggered the race and resulted in intermittent 500's and exceptions like the following being logged:

File "/Users/syn/projects/meltano/src/meltano/core/project_files.py", line 59, in include_paths
    include_path_patterns = self.meltano.get("include_paths", [])
AttributeError: 'NoneType' object has no attribute 'get'

Indicating that

self.meltano

has been reset to None. The only thing that should be setting

self._meltano = None

is `reset_cache`: meltano/src/meltano/core/project_files.py Lines 84 to 86 in</meltano/meltano/commit/859083b8c76c1b5d88ba30dbefb1b475a08f1fc4|859083b> | def reset_cache(self): | | ------------------------------------------------ | | """Reset cached view of the meltano.yml file.""" | | self._meltano = None | That in turn gets called in two spots

load

&

update

, meltano/src/meltano/core/project_files.py Lines 62 to 67 in</meltano/meltano/commit/859083b8c76c1b5d88ba30dbefb1b475a08f1fc4|859083b> | def load(self) -> dict: | | ------------------------------------------------------------------------ | | """Load all project files into a single dict representation.""" | | # meltano file may have changed in another process, so reset cache first | | self.reset_cache() | | included_file_contents = self._load_included_files() | | return deep_merge(self.meltano, included_file_contents) | meltano/src/meltano/core/project_files.py Lines 69 to 82 in</meltano/meltano/commit/859083b8c76c1b5d88ba30dbefb1b475a08f1fc4|859083b> | def update(self, meltano_config: dict): | | ------------------------------------------------------------------------------------------------------------------------ | | """Update config by overriding current config with new, changed config. | | | | Note:

.update()

will write blank entities for those no longer in use (i.e. contained config on load, but not on save). | | """ | | file_dicts = self._split_config_dict(meltano_config) | | for file_path, contents in file_dicts.items(): | | self._write_file(file_path, contents) | | | | unused_files = [fl for fl in self.include_paths if str(fl) not in file_dicts] | | for unused_file_path in unused_files: | | self._write_file(unused_file_path, BLANK_SUBFILE) | | self.reset_cache() | | return meltano_config | The load race is always present since the reset is unguarded and any concurrent calls via

self.project_files.load()

or

update()

could trigger it, and high request load or system latency would make it more likely to happen. And so, any concurrent calls to the path even when using the read lock can still kaboom. meltano/src/meltano/core/project.py Lines 235 to 236 in</meltano/meltano/commit/859083b8c76c1b5d88ba30dbefb1b475a08f1fc4|859083b> | with self._meltano_rw_lock.read_lock(): | | --------------------------------------------------- | | return MeltanoFile.parse(self.project_files.load()) | meltano/meltano