From: "Eregon (Benoit Daloze) via ruby-core" <ruby-core@ml.ruby-lang.org>
To: ruby-core@ml.ruby-lang.org
Cc: "Eregon (Benoit Daloze)" <noreply@ruby-lang.org>
Subject: [ruby-core:119254] [Ruby master Feature#20750] Expose ruby_thread_has_gvl_p in ruby/thread.h
Date: Wed, 18 Sep 2024 12:00:32 +0000 (UTC) [thread overview]
Message-ID: <redmine.journal-109834.20240918120031.12645@ruby-lang.org> (raw)
In-Reply-To: <redmine.issue-20750.20240917165251.12645@ruby-lang.org>
Issue #20750 has been updated by Eregon (Benoit Daloze).
OK the issue/bug is
https://github.com/ruby/ruby/blob/4797b0704ae49fb42c8ad9a45028efbe2298b5f5/thread.c#L1899
```
if (brb == 0) {
rb_bug("rb_thread_call_with_gvl: called by a thread which has GVL.");
}
```
@ko1 I think that's a bug, I think rb_thread_call_with_gvl() should be allowed if the GVL is already acquired, there seems to be no value to prevent it.
TruffleRuby already implements those semantics BTW.
How do you want to solve this issue?
----------------------------------------
Feature #20750: Expose ruby_thread_has_gvl_p in ruby/thread.h
https://bugs.ruby-lang.org/issues/20750#change-109834
* Author: kbrock (Keenan Brock)
* Status: Open
----------------------------------------
Hello All,
I'm hoping we can make `ruby_thread_has_gvl_p` a public method and no longer experimental.
I saw the following code in a gem that was not compiling with ruby 3.3:
``` c
// int ruby_thread_has_gvl_p(void); // <== line of concern
if (ruby_thread_has_gvl_p()) {
method_call(&context);
}
else {
rb_thread_call_with_gvl(method_call, &context);
}
```
400 unique projects on github added the line listed above to fix compilation. [1]
Some of the projects detected the method first in `extconf.rb`, but a majority just referenced it.
`ffi` used to have the detection code but dropped it since the method was in all supported ruby versions.
It feels like this method is now part of the undocumented public interface.
My suggestion is to add the method to the real public interface in `ruby/thread.h`.
PR with possible solution: https://github.com/ruby/ruby/pull/11619
Also included a patch if that is easier
Thank you for your consideration,
Keenan
Timeline:
- Dec 30 2008 - `rb_thread_call_with_gvl` introduced via 9c04a06 [2]
- Jan 12 2009 - `ruby_thread_has_gvl_p` introduced via 6f09fc2 [4]
- Jul 18 2012 - `rb_thread_call_with_gvl` made public via e9a91d2 [3]
Current references to code: `rb_thread_call_with_gvl` [5] `ruby_thread_has_gvl_p` [6]:
[1]: https://github.com/search?q=ruby_thread_has_gvl_p+language%3AC++NOT+is%3Afork++NOT+is%3Aarchived&type=code
[2]: https://github.com/ruby/ruby/commit/9c04a0647fb99f7554cb2e04385ddbb970631e02
[3]: https://github.com/ruby/ruby/commit/e9a91d2c95dfe22ad0487952f7a1053ef9a5fd16
[4]: https://github.com/ruby/ruby/commit/6f09fc2efd1915c4337ce6dded52a8a8771c3222
[5]: https://github.com/ruby/ruby/blob/master/thread.c#L1878
[6]: https://github.com/ruby/ruby/blob/master/thread.c#L1923
---Files--------------------------------
11618.patch (3.18 KB)
--
https://bugs.ruby-lang.org/
______________________________________________
ruby-core mailing list -- ruby-core@ml.ruby-lang.org
To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/
next prev parent reply other threads:[~2024-09-18 12:00 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-17 16:52 [ruby-core:119239] " kbrock (Keenan Brock) via ruby-core
2024-09-17 20:03 ` [ruby-core:119240] " Eregon (Benoit Daloze) via ruby-core
2024-09-17 22:34 ` [ruby-core:119241] " kbrock (Keenan Brock) via ruby-core
2024-09-18 11:56 ` [ruby-core:119253] " Eregon (Benoit Daloze) via ruby-core
2024-09-18 12:00 ` Eregon (Benoit Daloze) via ruby-core [this message]
2024-09-19 15:55 ` [ruby-core:119260] " kbrock (Keenan Brock) via ruby-core
2024-09-19 16:39 ` [ruby-core:119261] " kbrock (Keenan Brock) via ruby-core
2024-10-03 4:43 ` [ruby-core:119416] [Ruby master Feature#20750] Allow rb_thread_call_with_gvl to work when thread already has GVL matz (Yukihiro Matsumoto) via ruby-core
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=redmine.journal-109834.20240918120031.12645@ruby-lang.org \
--to=ruby-core@ml.ruby-lang.org \
--cc=noreply@ruby-lang.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).