jelmer / xandikos

A CalDAV/CardDAV server backed by Git
https://www.xandikos.org/
GNU General Public License v3.0
416 stars 42 forks source link

race conditions trigger exceptions #66

Open ecksun opened 6 years ago

ecksun commented 6 years ago

When I'm trying to synchronize my entire addressbook I get the following exceptions:

[pid: 12197|app: 0|req: 35/37] 192.168.1.1 () {50 vars in 714 bytes} [Sun Oct 15 13:07:54 2017] PROPFIND /user/contacts/addressbook/ => generated 287 bytes in 1 msecs (HTTP/1.1 207) 2 headers in 91 bytes (1 switches on core 0)
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/xandikos/webdav.py", line 1719, in __call__
    return do.handle(environ, start_response, self)
  File "/usr/lib/python3/dist-packages/xandikos/webdav.py", line 1423, in handle
    name, new_contents, content_type)
  File "/usr/lib/python3/dist-packages/xandikos/web.py", line 268, in create_member
    (name, etag) = self.store.import_one(name, content_type, contents)
  File "/usr/lib/python3/dist-packages/xandikos/store.py", line 399, in import_one
    etag = self._import_one(name, data, message, author=author)
  File "/usr/lib/python3/dist-packages/xandikos/store.py", line 791, in _import_one
    self.repo.stage(name)
  File "/usr/lib/python3/dist-packages/dulwich/repo.py", line 867, in stage
    index.write()
  File "/usr/lib/python3/dist-packages/dulwich/index.py", line 217, in write
    f = GitFile(self._filename, 'wb')
  File "/usr/lib/python3/dist-packages/dulwich/file.py", line 87, in GitFile
    return _GitFile(filename, mode, bufsize)
  File "/usr/lib/python3/dist-packages/dulwich/file.py", line 112, in __init__
    os.O_RDWR | os.O_CREAT | os.O_EXCL | getattr(os, "O_BINARY", 0))
FileExistsError: [Errno 17] File exists: '/var/lib/xandikos/collections/user/contacts/.git/index.lock'
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/xandikos/webdav.py", line 1719, in __call__
    return do.handle(environ, start_response, self)
  File "/usr/lib/python3/dist-packages/xandikos/webdav.py", line 1382, in handle
    unused_href, path, r = app._get_resource_from_environ(environ)
  File "/usr/lib/python3/dist-packages/xandikos/webdav.py", line 1680, in _get_resource_from_environ
    r = self.backend.get_resource(path)
  File "/usr/lib/python3/dist-packages/xandikos/web.py", line 752, in get_resource
    return store.get_member(name)
  File "/usr/lib/python3/dist-packages/xandikos/web.py", line 249, in get_member
    for (fname, content_type, fetag) in self.store.iter_with_etag():
  File "/usr/lib/python3/dist-packages/xandikos/store.py", line 455, in iter_with_etag
    for (name, mode, sha) in self._iterblobs(ctag):
  File "/usr/lib/python3/dist-packages/xandikos/store.py", line 841, in _iterblobs
    index = self.repo.open_index()
  File "/usr/lib/python3/dist-packages/dulwich/repo.py", line 826, in open_index
    return Index(self.index_path())
  File "/usr/lib/python3/dist-packages/dulwich/index.py", line 206, in __init__
    self.read()
  File "/usr/lib/python3/dist-packages/dulwich/index.py", line 235, in read
    f.check_sha()
  File "/usr/lib/python3/dist-packages/dulwich/pack.py", line 1391, in check_sha
[pid: 12198|app: 0|req: 9/55] 192.168.1.1 () {48 vars in 735 bytes} [Sun Oct 15 13:09:24 2017] PUT /user/contacts//2170d417bab64d4aa4994ddd975eaa4a.vcf => generated 0 bytes in 10 msecs (HTTP/1.1 201) 1 headers in 74 bytes (0 switches on core 0)
    raise ChecksumMismatch(self.sha1.hexdigest(), sha_to_hex(stored))
  File "/usr/lib/python3/dist-packages/dulwich/objects.py", line 85, in sha_to_hex
    assert len(hexsha) == 40, "Incorrect length of sha1 string: %d" % hexsha
TypeError: %d format: a number is required, not bytes
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/xandikos/store.py", line 595, in get_type
    store_type = config.get(b'xandikos', b'type')
  File "/usr/lib/python3/dist-packages/dulwich/config.py", line 147, in get
[pid: 12198|app: 0|req: 24/90] 192.168.1.1 () {48 vars in 735 bytes} [Sun Oct 15 13:10:36 2017] PUT /user/contacts//fc23c5ba75fa49eda43771958bd381ad.vcf => generated 0 bytes in 24 msecs (HTTP/1.1 201) 1 headers in 74 bytes (0 switches on core 0)
    return self._values[(section[0],)][name]
KeyError: (b'xandikos',)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/xandikos/webdav.py", line 1719, in __call__
    return do.handle(environ, start_response, self)
  File "/usr/lib/python3/dist-packages/xandikos/webdav.py", line 1414, in handle
    r = app.backend.get_resource(container_path)
  File "/usr/lib/python3/dist-packages/xandikos/web.py", line 742, in get_resource
    }[store.get_type()](self, relpath, store)
  File "/usr/lib/python3/dist-packages/xandikos/store.py", line 597, in get_type
    return super(GitStore, self).get_type()
  File "/usr/lib/python3/dist-packages/xandikos/store.py", line 253, in get_type
    for (name, content_type, etag) in self.iter_with_etag():
  File "/usr/lib/python3/dist-packages/xandikos/store.py", line 455, in iter_with_etag
    for (name, mode, sha) in self._iterblobs(ctag):
  File "/usr/lib/python3/dist-packages/xandikos/store.py", line 841, in _iterblobs
    index = self.repo.open_index()
  File "/usr/lib/python3/dist-packages/dulwich/repo.py", line 826, in open_index
    return Index(self.index_path())
  File "/usr/lib/python3/dist-packages/dulwich/index.py", line 206, in __init__
    self.read()
  File "/usr/lib/python3/dist-packages/dulwich/index.py", line 235, in read
    f.check_sha()
  File "/usr/lib/python3/dist-packages/dulwich/pack.py", line 1391, in check_sha
    raise ChecksumMismatch(self.sha1.hexdigest(), sha_to_hex(stored))
  File "/usr/lib/python3/dist-packages/dulwich/objects.py", line 85, in sha_to_hex
    assert len(hexsha) == 40, "Incorrect length of sha1 string: %d" % hexsha
TypeError: %d format: a number is required, not bytes

All of them happens more than once.

I'm using the following configuration:

version: 0.0.6-1~bpo9+1 (debian stretch)

nginx config:

server {
    include /etc/nginx/includes/https;
    include /etc/nginx/includes/common;

    server_name dav.ecksun.com;

    # certs sent to the client in SERVER HELLO are concatenated in ssl_certificate
    ssl_certificate /etc/letsencrypt/live/www.ecksun.com/fullchain.pem;
    ssl_certificate_key /etc/letsencrypt/live/www.ecksun.com/privkey.pem;

    ## verify chain of trust of OCSP response using Root CA and Intermediate certs
    ssl_trusted_certificate /etc/letsencrypt/live/www.ecksun.com/fullchain.pem;

    location / {
        auth_basic            "Restricted";
        auth_basic_user_file  /var/www/htpasswd/xandikos;

        include uwsgi_params;
        uwsgi_param SCRIPT_NAME     "";
        uwsgi_pass unix:///tmp/xandikos.sock;
        uwsgi_param AUTOCREATE "defaults";
    }
}

and uwsgi config:

[uwsgi]
socket = /tmp/xandikos.sock
uid = xandikos
gid = xandikos
master = true
cheaper = 2
processes = 4
plugin = python3
module = xandikos.wsgi:app
umask = 022
env = XANDIKOSPATH=/var/lib/xandikos/collections
env = CURRENT_USER_PRINCIPAL=/user/
# Set AUTOCREATE to have Xandikos create default CalDAV/CardDAV
# collections if they don't yet exist. Possible values:
#  - principal: just create the current user principal
#  - defaults: create the principal and default calendar and contacts
#       collections. (recommended)
env = AUTOCREATE=defaults
env = XANDIKOS_DUMP_DAV_XML=1
ecksun commented 6 years ago

Running the same config but with 931c149123204189de26265b382b93631b675001 and uwsgi like:

uwsgi --socket /tmp/xandikos.sock --need-plugin python3 --module=xandikos.wsgi:app --virtualenv ./venv/

Did not exhibit the same issues

ecksun commented 6 years ago

Running it again with master, but with this uwsgi config:

[uwsgi]
socket = /tmp/xandikos.sock
uid = xandikos
gid = xandikos
master = true
cheaper = 2
processes = 4
plugin = python3
module = xandikos.wsgi:app
umask = 022
chmod-socket = 660
virtualenv = /var/lib/xandikos/xandikos/xandikos/venv
env = XANDIKOSPATH=/var/lib/xandikos/collections
env = CURRENT_USER_PRINCIPAL=/ecksun/
# Set AUTOCREATE to have Xandikos create default CalDAV/CardDAV
# collections if they don't yet exist. Possible values:
#  - principal: just create the current user principal
#  - defaults: create the principal and default calendar and contacts
#       collections. (recommended)
env = AUTOCREATE=defaults
env = XANDIKOS_DUMP_DAV_XML=1

I get the errors again, I'm thinking its related to processes = 4

ecksun commented 6 years ago

Changing to this:

cheaper = 0
processes = 1

Seems to solve the issues.

So my question is then, is xandikos and its git libraries built for parallel use?

jelmer commented 6 years ago

This is working correctly in that xandikos/dulwich catch the race, and throw an exception.

What's missing is that Xandikos needs to act gracefully whenever this race condition happens. Either it can return an error to the client, or it can try again (assuming the file it's trying to change wasn't involved in the racing changes).

ecksun commented 6 years ago

I created a pull-request to update the examples just to simplify for people like me, until a real solution can be implemented