Issue: Race condition at class level when dynamically binding the DB
Author: whisust
State: closed
Created: 2021-02-10 09:51:18

Related to https://github.com/coleifer/peewee/issues/2339 and in a minor way https://github.com/coleifer/peewee/issues/2306

# Context
Usage of peewee with
- two connection pools to the same database, one being restricted in read-only (read replica or read-only user)
- usage in a greenlet environment (here gunicorn running gevent greenlets)

# Run Explanation
We select the database dynamically depending on the query to run.
A read-only query will be run using connections from the read-able pools;
A write query will be run only on the leader pool.
Thus, before each query, we `model.bind(db)` using the correct db

# Issue
With the database being tied to the Model class after the `.bind`, plus the Model class reference being shared by both greenlets, the Model._meta.database becomes a mutable, shared, non-synchronized data field.

Thus, it's possible to create a race condition if at least two greenlets are doing read and write operations (and so, db bindings) simultaneously on the same model.

It is highly unlikely, but under a basic load (let's say a b2c website with a sustained traffic) it happens. In my case, with 1 million requests per day, it happened 5-10 times in 24h.

# Propositions
- Workaround (me): adding a class-level lock to ensure no db re-bind are done between a db<->model bind and the query execution for write operations in the thread scope [quickfix, hacky, can cause perf issues? but usable]
- Upgrade (peewee): proposing a way to select the DB at query time instead of taking the hardbound one (like a `database_provider` at class level instead of the `database`, taking in argument at least the read-only indication as query context). 
For this one, I know there is already a `database` optional argument in the `execute` method. I can use it if that's your recommended way, but my question would be with the overall coherence of the peewee api's (mix between the presets like `get_or_none` that cannot be customised, and the `Model.select().execute(db?)`.
- Upgrade (peewee): not binding the model to a db, or not binding the db and the connection <- huge API change I think

 Here is an example using a Sentry stack trace, and keep in mind that it's used in a multithreaded environment:

![Screenshot 2021-02-10 at 10 20 23](https://user-images.githubusercontent.com/22323709/107492871-36ddd300-6b8d-11eb-8d15-4f9392ac8b4e.png)

![Screenshot 2021-02-10 at 10 20 50](https://user-images.githubusercontent.com/22323709/107492888-3c3b1d80-6b8d-11eb-8bc5-27285fa2ed52.png)


I can do a code snippet to reproduce the error if you want.


>We select the database dynamically depending on the query to run.
>A read-only query will be run using connections from the read-able pools;
>A write query will be run only on the leader pool.
>Thus, before each query, we model.bind(db) using the correct db

You might try storing `Model._meta.database` in a thread-local. I tested and this is working fine for me:

```python
from peewee import Metadata

class ThreadSafeDatabaseMetadata(Metadata):
    def __init__(self, *args, **kwargs):
        self._local = threading.local()
        super(ThreadSafeDatabaseMetadata, self).__init__(*args, **kwargs)

    def _get_db(self):
        return getattr(self._local, 'database', self._database)
    def _set_db(self, db):
        self._local.database = self._database = db
    database = property(_get_db, _set_db)

class BaseModel(Model):
    class Meta:
        model_metadata_class = ThreadSafeDatabaseMetadata
```

Overall I do not plan on changing the way binding databases works for the models API.