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

Implement Fiber#raise #2338

Closed
eregon opened this issue Apr 28, 2021 · 8 comments
Closed

Implement Fiber#raise #2338

eregon opened this issue Apr 28, 2021 · 8 comments
Assignees
Milestone

Comments

@eregon
Copy link
Member

eregon commented Apr 28, 2021

It's part of the new 2.7 methods: #2004
And it's used notably by socketry/io-event#2
cc @ioquatix

@ioquatix
Copy link
Contributor

Thanks, and also allow Fiber#transfer and Fiber#resume/yield interleaved.

@eregon
Copy link
Member Author

eregon commented Apr 29, 2021

The change for Fiber#transfer allowed in more cases is only in Ruby 3.0+ though, right?

@bjfish bjfish self-assigned this Apr 29, 2021
@ioquatix
Copy link
Contributor

The change for Fiber#transfer allowed in more cases is only in Ruby 3.0+ though, right?

It is purely additive, it won't break Ruby 2.x

@eregon
Copy link
Member Author

eregon commented Apr 30, 2021

Looking at Fiber specs, it seems there would be specs that break if we change the behavior of Fiber#transfer while reporting RUBY_VERSION as 2.7:
https://github.com/oracle/truffleruby/blob/master/spec/ruby/library/fiber/transfer_spec.rb
https://github.com/oracle/truffleruby/blob/master/spec/ruby/library/fiber/resume_spec.rb

Is it needed by https://github.com/socketry/event ?

@eregon eregon self-assigned this Apr 30, 2021
@ioquatix
Copy link
Contributor

ioquatix commented May 2, 2021

The event library needs the following:

  • Fiber#raise (for cancellation)
  • Fiber#transfer (for non-blocking operation)

For new version of async (currently unreleased prototype) which depends on event, Fiber#resume and Fiber#yield is not used. It's purely for user code. Depending on how you implemented lazy enumerator, this may be a problem. From user point of view, semantics remain unchanged.

@eregon eregon added this to the 21.2.0 milestone May 3, 2021
@eregon
Copy link
Member Author

eregon commented May 3, 2021

Fiber#raise was implemented in 52f7d8b by @bjfish.
If we need to allow more cases for transfer to pass tests/specs of event or async, could you create a separate issue?

@eregon eregon closed this as completed May 3, 2021
@eregon
Copy link
Member Author

eregon commented May 3, 2021

Looks like the event specs need Fiber#raise to be allowed after Fiber#transfer: socketry/io-event#5 (comment)
That's a change of Ruby 3 as well: https://github.com/ruby/ruby/pull/3795/files#diff-f5e06781d15d3a6c16eef4cb49eebc310820bead3672ed84608c84958ae5da14R2341-R2343
=> #2342

@ioquatix
Copy link
Contributor

ioquatix commented May 4, 2021

Yes, Fiber#raise must do the right thing in order to transfer back to the fiber.

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

No branches or pull requests

3 participants