Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Race condition when waiting on Promise resolved in other thread #87

Open
Novakov opened this issue Jan 18, 2020 · 5 comments · May be fixed by #89
Open

Race condition when waiting on Promise resolved in other thread #87

Novakov opened this issue Jan 18, 2020 · 5 comments · May be fixed by #89

Comments

@Novakov
Copy link

Novakov commented Jan 18, 2020 •
edited

I'm using with LibUsb to achieve asynchronous transfers. Without going into too much details I'm using two threads:

  1. Worker: creates promise (P) and starts async USB transfer with supplied callback (C) that resolves promise. After starting transfer, thread wait for promise result.
  2. LibUSB event loop: triggers event processing in LibUSB, which will call callback C which will resolve promise and resume Worker thread.

All calls to LibUSB are native calls (ctypes).

Occasionally (one in hundreds) I'm getting AssertionFailed exception on this line: https://github.com/syrusakbary/promise/blob/master/promise/promise.py#L414 with traceback:

Traceback (most recent call last):
  File "C:\Python38\lib\threading.py", line 932, in _bootstrap_inner
    self.run()
  File "C:\Python38\lib\threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "<snip>", line 66, in _run
    packet = self._segmentation.read_packet()
  File "<snip>", line 60, in read_packet
    payload = self._low.low_read(part_size)
  File "<snip>.py", line 56, in low_read
    return bytes(ret.get())
  File "C:\Python38\lib\site-packages\promise\promise.py", line 511, in get
    self._wait(timeout or DEFAULT_TIMEOUT)
  File "C:\Python38\lib\site-packages\promise\promise.py", line 506, in _wait
    self.wait(self, timeout)
  File "C:\Python38\lib\site-packages\promise\promise.py", line 502, in wait
    async_instance.wait(promise, timeout)
  File "C:\Python38\lib\site-packages\promise\async_.py", line 117, in wait
    target.scheduler.wait(target, timeout)
  File "C:\Python38\lib\site-packages\promise\schedulers\immediate.py", line 24, in wait
    promise._then(on_resolve_or_reject, on_resolve_or_reject)
  File "C:\Python38\lib\site-packages\promise\promise.py", line 577, in _then
    target._add_callbacks(did_fulfill, did_reject, promise)
  File "C:\Python38\lib\site-packages\promise\promise.py", line 414, in _add_callbacks
    assert not self._rejection_handler0"
AssertionError

Traceback indicates that assertion failed in context on Worker thread.

I was able to create (sort of...) reproduction of this issue: https://github.com/Novakov/promise/tree/race-condition Unfortunately it's not great. Running test https://github.com/Novakov/promise/blob/race-condition/tests/test_thread_safety.py#L122 shows the issue in most cases.

Investigation of Promise state suggests that in line https://github.com/syrusakbary/promise/blob/master/promise/promise.py#L577 promise is still pending, than context switch happens, promise gets resolved in loop thread, context switch happends again and _add_callbacks continues on now resolved promise.

Machine details
OS: Windows 10 x64
CPU: 4 cores, 8 logical threads
Python: 3.8.1 (tags/v3.8.1:1b293b6, Dec 18 2019, 23:11:46) [MSC v.1916 64 bit (AMD64)]

I was not able to reproduce the issue on Linux machine, however it maybe matter of probability
@syrusakbary
Copy link
Owner

What version of Promise are you using? It seems what you commented should be solved in 2.3.0

@Novakov
Copy link
Author

Novakov commented Jan 29, 2020

I forked this repository and added test to it. Tag matching release 2.3.0 is there.
Latest commit: Novakov@9ba6fbd (2020-01-08)

@Zahlii
Copy link

Zahlii commented Apr 5, 2020

Piggybacking on this issue - I am also getting this error and made sure that I had Promise>=2.3.0 installed prior. Sadly, it is not easily reproducible. The general setting however is the following:

I am calling functions using a ThreadPoolExecutor, and each of them returns a Promise. Now, I use Promise.all().then(lambda res: pool.submit(somefunction, res)).

This means I am using Promise to handle my threadpool-based Task executing and scheduling.

@alex-verve
Copy link

Also getting these errors (14 times per 2.5 million requests)

  File "/var/task/graphql/execution/executor.py", line 218, in complete_value_catching_error
    completed = complete_value(exe_context, return_type, field_asts, info, result)
, 
  File "/var/task/graphql/execution/executor.py", line 262, in complete_value
    lambda error: Promise.rejected(GraphQLLocatedError(field_asts, original_error=error))
, 
  File "/var/task/promise/promise.py", line 630, in then
    return self._then(did_fulfill, did_reject)
, 
  File "/var/task/promise/promise.py", line 577, in _then
    target._add_callbacks(did_fulfill, did_reject, promise)
, 
  File "/var/task/promise/promise.py", line 414, in _add_callbacks
    assert not self._rejection_handler0

We're executing Threads with callbacks which resolve promise (similar to @Novakov case)

alex-verve pushed a commit to alex-verve/promise that referenced this issue Jul 15, 2020
Add unit test;
3e69904 
credits to @Novakov at syrusakbary#87
@alex-verve alex-verve linked a pull request Jul 15, 2020 that will close this issue
Open
alex-verve pushed a commit to alex-verve/promise that referenced this issue Jul 15, 2020
Add unit test;
e98f616 
credits to @Novakov at syrusakbary#87
alex-verve pushed a commit to alex-verve/promise that referenced this issue Jul 15, 2020
Add unit test;
61f0b86 
credits to @Novakov at syrusakbary#87
alex-verve pushed a commit to alex-verve/promise that referenced this issue Jul 15, 2020
Add unit test;
35b9845 
credits to @Novakov at syrusakbary#87
alex-verve pushed a commit to alex-verve/promise that referenced this issue Jul 15, 2020
Add thread-safe lock for _state and _rejection_handler0 modifications
7ae2c1c 
fixes syrusakbary#87
alex-verve pushed a commit to alex-verve/promise that referenced this issue Jul 16, 2020
Add unit test;
65f723e 
credits to @Novakov at syrusakbary#87
alex-verve pushed a commit to alex-verve/promise that referenced this issue Jul 16, 2020
Add thread-safe lock for _state and _*_handler's modifications
a57cd4c 
fixes syrusakbary#87
@alex-verve
Copy link

I think I fixed the problem. Review is welcome #89

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix race condition when pending Promise was resolved in other thread alex-verve/promise
4 participants
@syrusakbary @Zahlii @Novakov @alex-verve