web-platform-tests / wpt

Test suites for Web platform specs — including WHATWG, W3C, and others
https://web-platform-tests.org/
Other
4.86k stars 3.03k forks source link

request.server.stash.lock not working? #28662

Closed ArthurSonzogni closed 3 years ago

ArthurSonzogni commented 3 years ago

In bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1198255

I have issue with the file: html/cross-origin-embedder-policy/credentialless/resources/dispatcher.py

It can be summarized by:

def main(request, response):
    # [...]
    stash = request.server.stash
    # The stash is accessed concurrently by many clients. A lock is used to
    # avoid interleaved read/write from different clients.
    with stash.lock:
        queue = stash.take(uuid) or []
        # [...] Modify |queue|.
        stash.put(uuid, queue)
    return ret;

Theoricaly, every stash.put() is preceded by stash.take(). stash.lock() prevents concurrent access.

Despite this, there are errors sometimes: https://github.com/web-platform-tests/wpt/pull/28575/checks?check_run_id=2391872015

Traceback (most recent call last):
File \"/home/test/web-platform-tests/tools/wptserve/wptserve/handlers.py\", line 362, in __call__
rv = self.func(request, response)
File \"/home/test/web-platform-tests/html/cross-origin-embedder-policy/credentialless/resources/dispatcher.py\", line 48, in main
stash.put(uuid, queue)
File \"/home/test/web-platform-tests/tools/wptserve/wptserve/stash.py\", line 177, in put
raise StashError(\"Tried to overwrite existing shared stash value \"
wptserve.stash.StashError: Tried to overwrite existing shared stash value for key (b'/html/cross-origin-embedder-policy/credentialless/resources/dispatcher.py', b'\\xb0q@h\\xfcL\\x1b\\xb8E\\xfd\\xe3U\\xe1\\xca\\xa5') (old value was [b'done'], new value is [])

This sounds like a bug in stash.lock.

@stephenmcgruer Do you expect this code to be correct? Or there is something obviously wrong I don't understand? I have seen you made many changes about stash in the recent past.

stephenmcgruer commented 3 years ago

@stephenmcgruer Do you expect this code to be correct? Or there is something obviously wrong I don't understand? I have seen you made many changes about stash in the recent past.

I'm afraid I no longer work on WPT and do not have time to investigate this problem. cc @web-platform-tests/wpt-core-team

ArthurSonzogni commented 3 years ago

Thanks for you help so far!

foolip commented 3 years ago

@ArthurSonzogni when this happens, are there multiple instances of wptserve running this test at the same time, or does it happen without anything happen concurrently at all?

ArthurSonzogni commented 3 years ago

I am not sure. I don't see any process/thread with this name. I guess this happens within python3, so I would say yes, given the output:

ps -avt | grep /usr/bin/python3
0.0 /usr/bin/python3 -c from multiprocessing.resource_tracker import main;main(5)
0.0 /usr/bin/python3 -c from multiprocessing.spawn import spawn_main; spawn_main(tracker_fd=6, pipe_handle=10) --multiprocessing-fork
0.1 /usr/bin/python3 -c from multiprocessing.spawn import spawn_main; spawn_main(tracker_fd=6, pipe_handle=12) --multiprocessing-fork
0.1 /usr/bin/python3 -c from multiprocessing.spawn import spawn_main; spawn_main(tracker_fd=6, pipe_handle=14) --multiprocessing-fork
0.1 /usr/bin/python3 -c from multiprocessing.spawn import spawn_main; spawn_main(tracker_fd=6, pipe_handle=16) --multiprocessing-fork
0.1 /usr/bin/python3 -c from multiprocessing.spawn import spawn_main; spawn_main(tracker_fd=6, pipe_handle=18) --multiprocessing-fork
0.1 /usr/bin/python3 -c from multiprocessing.spawn import spawn_main; spawn_main(tracker_fd=6, pipe_handle=20) --multiprocessing-fork
0.1 /usr/bin/python3 -c from multiprocessing.spawn import spawn_main; spawn_main(tracker_fd=6, pipe_handle=22) --multiprocessing-fork
0.1 /usr/bin/python3 -c from multiprocessing.spawn import spawn_main; spawn_main(tracker_fd=6, pipe_handle=24) --multiprocessing-fork
0.1 /usr/bin/python3 -c from multiprocessing.spawn import spawn_main; spawn_main(tracker_fd=6, pipe_handle=26) --multiprocessing-fork
0.1 /usr/bin/python3 -c from multiprocessing.spawn import spawn_main; spawn_main(tracker_fd=6, pipe_handle=35) --multiprocessing-fork

This is mostly failing during stability test. I am using:

./wpt run --repeat 20 --binary=/usr/bin/google-chrome-unstable chrome ./html/cross-origin-embedder-policy/credentialless/cors-or-credentialless/redirect.tentative.html

and can reliably reproduce every 10 runs.