edgedb-python icon indicating copy to clipboard operation
edgedb-python copied to clipboard

Forking process with edgedb connection is dangerous

Open tailhook opened this issue 4 years ago • 4 comments

In particular, if connection is used in both forks, it looks like this:

    Error on request:
    Traceback (most recent call last):
      File "/usr/local/lib/python3.6/dist-packages/werkzeug/serving.py", line 323, in run_wsgi
        execute(self.server.app)
      File "/usr/local/lib/python3.6/dist-packages/werkzeug/serving.py", line 312, in execute
        application_iter = app(environ, start_response)
      File "/usr/local/lib/python3.6/dist-packages/werkzeug/wrappers/base_request.py", line 238, in application
        resp = f(*args[:-2] + (request,))
      File "/work/serve_py_sync/main.py", line 48, in application
        return ROUTES[endpoint](request, **values)
      File "/work/serve_py_sync/main.py", line 39, in increment
        """, name=name)
      File "/usr/local/lib/python3.6/dist-packages/edgedb/blocking_con.py", line 117, in query_one
        io_format=protocol.IoFormat.BINARY,
      File "edgedb/protocol/blocking_proto.pyx", line 91, in edgedb.protocol.blocking_proto.BlockingIOProtocol.sync_execute_anonymous

      File "edgedb/protocol/blocking_proto.pyx", line 77, in edgedb.protocol.blocking_proto.BlockingIOProtocol._iter_coroutine

      File "edgedb/protocol/protocol.pyx", line 588, in execute_anonymous

      File "edgedb/protocol/protocol.pyx", line 269, in _parse

      File "edgedb/protocol/protocol.pyx", line 1057, in edgedb.protocol.protocol.SansIOProtocol.fallthrough

    edgedb.errors.ProtocolError: unexpected message type 'T'Error on request:
    Traceback (most recent call last):
      File "/usr/local/lib/python3.6/dist-packages/werkzeug/serving.py", line 323, in run_wsgi
        execute(self.server.app)
      File "/usr/local/lib/python3.6/dist-packages/werkzeug/serving.py", line 312, in execute
        application_iter = app(environ, start_response)
      File "/usr/local/lib/python3.6/dist-packages/werkzeug/wrappers/base_request.py", line 238, in application
        resp = f(*args[:-2] + (request,))
      File "/work/serve_py_sync/main.py", line 48, in application
        return ROUTES[endpoint](request, **values)
      File "/work/serve_py_sync/main.py", line 39, in increment
        """, name=name)
      File "/usr/local/lib/python3.6/dist-packages/edgedb/blocking_con.py", line 117, in query_one
        io_format=protocol.IoFormat.BINARY,
      File "edgedb/protocol/blocking_proto.pyx", line 91, in edgedb.protocol.blocking_proto.BlockingIOProtocol.sync_execute_anonymous

      File "edgedb/protocol/blocking_proto.pyx", line 77, in edgedb.protocol.blocking_proto.BlockingIOProtocol._iter_coroutine

      File "edgedb/protocol/protocol.pyx", line 588, in execute_anonymous

      File "edgedb/protocol/protocol.pyx", line 310, in _parse

      File "edgedb/protocol/protocol.pyx", line 1057, in edgedb.protocol.protocol.SansIOProtocol.fallthrough

    edgedb.errors.ProtocolError: unexpected message type '1'

So the issue is similar to #130.

We may use os.register_at_fork to fix this in python3.7.

tailhook avatar Dec 29 '20 16:12 tailhook

How do other database drivers handle this?

elprans avatar Dec 29 '20 17:12 elprans

How do other database drivers handle this?

Yeah, good question.

1st1 avatar Jan 05 '21 03:01 1st1

psycopg2/libpq simply doesn't support forking, but they are connection-based, not pool-based like the EdgeDB client. I looked into a few other drivers and seems like "fork" is a very rare keyword as in multiprocessing (very ocassionally someone complains postgres-rust doesn't work in Celery subprocesses) - I'd say they don't support forking.

Theoretically we could shutdown all pooled connections in a subprocess after forking, but there're also very marginal cases to take care of like forking happened during a query or transaction.

fantix avatar May 25 '23 18:05 fantix

there're also very marginal cases to take care of like forking happened during a query or transaction.

The error above and very apparent security issue (i.e. wrong reply matched to a request) happens exactly in this case. So we have to choose which process these connections can continue in. Which is I think is the parent process. In other ones it's okay to return an error.

I'm not sure whether we want to throw a specific error or just some "ConnectionClosedError" in this case. Because I think this is very Python-specific issue. I think most other languages do not support full-featured fork (C/C++ being rare exception, but we don't have C bindings yet).

tailhook avatar May 29 '23 11:05 tailhook