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

epoll instance is dead #61

Closed
emixa-d opened this issue Jun 30, 2022 · 6 comments · Fixed by #62
Closed

epoll instance is dead #61

emixa-d opened this issue Jun 30, 2022 · 6 comments · Fixed by #62

Comments

@emixa-d
Copy link
Collaborator

emixa-d commented Jun 30, 2022

Currently minimising my test case ...

@emixa-d
Copy link
Collaborator Author

emixa-d commented Jun 30, 2022

Minimal test case:

(use-modules (fibers) (fibers conditions) (fibers scheduler))

(define c (make-condition))
(run-fibers
 (lambda ()
   (spawn-fiber (lambda () (wait c)))
   (yield-current-task)) ; let the other fiber wait forever
 #:hz 0 #:parallelism 1) ; it fails both with and without, preemption is another complication, so keep things simple for now by not preempting

(signal-condition! c)

@emixa-d
Copy link
Collaborator Author

emixa-d commented Jun 30, 2022

Description:

When a fiber is waiting on a condition and run-fibers completes before the fiber completes and afterwards the condition is signalled, ‘epoll instance is dead’ results.

@emixa-d
Copy link
Collaborator Author

emixa-d commented Jun 30, 2022

Proposed fix: change

    ('dead (error "epoll instance is dead"))))

to

    ;; This can happen if a fiber was waiting on a condition and run-fibers completes before
    ;; the fiber completes and afterwards the condition is signalled.  In that case, we don't
    ;; have to resurrect the fiber or something, we can just do nothing.
    ;; (Bug report: https://github.com/wingo/fibers/issues/61)
    ('dead #t)))

and add the test to the test suite?

(Edit: clarification: I did not test this.)

@wingo
Copy link
Owner

wingo commented Jun 30, 2022

Sounds like a good fix to me!

@aconchillo
Copy link
Collaborator

Sounds good to me too. @emixa-d can you provide a PR for this? i'll merge right away. thanks!

emixa-d added a commit that referenced this issue Jun 30, 2022
This avoids the "epoll instance is dead" error noticed in
GNUnet-Scheme's test suite, as reported at
<#61>.
A test is added in the next commit.

* fibers/epoll.scm (epoll-wake!)[dead]: Instead of throwing an error,
just return #t.
emixa-d added a commit that referenced this issue Jun 30, 2022
* tests/conditions.scm: Add a test.
@emixa-d
Copy link
Collaborator Author

emixa-d commented Jun 30, 2022

I've made a PR at: #62

aconchillo added a commit that referenced this issue Jun 30, 2022
Fix ‘epoll instance is dead #61
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

Successfully merging a pull request may close this issue.

3 participants