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.

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

Assert in pony_asio_event_unsubscribe fired #2580

Closed
winksaville opened this issue Mar 7, 2018 · 9 comments
Closed

Assert in pony_asio_event_unsubscribe fired #2580

winksaville opened this issue Mar 7, 2018 · 9 comments

Comments

@winksaville
Copy link
Contributor

TravisCI failed in MacOS test @slfritchie, could this be related to #2561?

238 tests started, 238 complete: process/WritevOrdering complete
239 tests started, 238 complete: process/PrintvOrdering started
src/libponyrt/asio/kqueue.c:377: pony_asio_event_unsubscribe: Assertion 0 failed.

Backtrace:
0 stdlib 0x00000001003b4411 ponyint_assert_fail + 161
1 stdlib 0x00000001003a1a98 pony_asio_event_unsubscribe + 136
2 stdlib 0x0000000100069ed2 process_ProcessMonitor_ref__close_fd_Io + 114
3 stdlib 0x000000010039e8e2 ponyint_actor_run + 162
4 stdlib 0x00000001003b616b run + 251
5 stdlib 0x00000001003b5849 run_thread + 57
6 libsystem_pthread.dylib 0x00007fffeb18a93b _pthread_body + 180
7 libsystem_pthread.dylib 0x00007fffeb18a887 _pthread_body + 0
8 libsystem_pthread.dylib 0x00007fffeb18a08d thread_start + 13
make: *** [test-ci] Abort trap: 6

travis_time:end:02b4d2f7:start=1520398511475509000,finish=1520398885530599000,duration=374055090000
�[0K

�[31;1mThe command "bash .travis_script.bash" exited with 2.�[0m

Done. Your build exited with 1.

@slfritchie
Copy link
Contributor

Ouch, I hope not, @winksaville.

I wonder if it's related to the race(s) described in #2529. The lost wakeup event work in #2561 wasn't meant to fix ProcessMonitor ... but I suppose that it's possible that #2561 makes the ProcessMonitor problem worse (or more noticable or ... some other adjective)?

cc: @dipinhora

@winksaville
Copy link
Contributor Author

@slfritchie, I'm guessing my changes to gbenchmark didn't cause it :)

@mfelsche
Copy link
Contributor

mfelsche commented Mar 7, 2018

This seems to be related to #2574 which was recently merged.

@clearyf
Copy link
Contributor

clearyf commented Mar 7, 2018

Right, so it appears that in the kqueue code there is an assert that triggers if the event is unsubscribed more than once. I don't have a Mac system so I couldn't test this here myself, but I can update the ProcessMonitor code to only unsubscribe once.

@clearyf
Copy link
Contributor

clearyf commented Mar 7, 2018

79ed140 should fix the problem; on Linux there is no assert to prevent against multiple unsubscribes, instead the epoll implementation just silently exits early instead of triggering an assert. The problem is that after the first _close_fd the fd is then set to -1, and that then matches at least one of the cases when all fds are closed in _close, unsubscribing a second time.
Q: Should the epoll and iocp backends also trigger an assert?

@mfelsche
Copy link
Contributor

mfelsche commented Mar 7, 2018

@clearyf could you create a PR with your commit? It makes sense regardless of the question of adding asserts (which should be considered, imo).

@clearyf
Copy link
Contributor

clearyf commented Mar 7, 2018

Done in #2581. Changing the linux & windows asio implementations to assert is easy but might break existing code, in which case it's best to do it asap.

@mfelsche
Copy link
Contributor

mfelsche commented Mar 7, 2018

With merging of #2581 this problem should be solved.

@clearyf i nonetheless think it should be tried to add assertions to linux and windows code and see how we run into them there and if this is a design flaw of the stdlib.

@winksaville
Copy link
Contributor Author

Closing with the hope that #2581 resolves the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants