Skip to content

Conversation

@eregon
Copy link
Member

@eregon eregon commented Feb 19, 2025

I think this makes sense for now to address the segfault.
Of course it can be improved later to use explicit locking.

@eregon eregon requested a review from nobu February 19, 2025 16:47
@nobu
Copy link
Member

nobu commented Mar 18, 2025

I think methods from Etc.sysconfdir to Etc.nprocessors are ractor-safe.

@eregon
Copy link
Member Author

eregon commented Mar 23, 2025

I think methods from Etc.sysconfdir to Etc.nprocessors are ractor-safe.

Agreed, so I marked those as Ractor-safe and the rest as Ractor-unsafe, and added tests.

ext/etc/etc.c Outdated
Comment on lines 1198 to 1201
rb_define_module_function(mEtc, "uname", etc_uname, 0);
rb_define_module_function(mEtc, "sysconf", etc_sysconf, 1);
rb_define_module_function(mEtc, "confstr", etc_confstr, 1);
rb_define_method(rb_cIO, "pathconf", io_pathconf, 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these 3 are probably MT-safe, and possibly also pathconf.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I'll add them.
And systmpdir seems fine too, all functions it calls take a buffer (for the path) so should be thread-safe.

I'll add a test calling these functions in parallel to try to ensure they are indeed thread/Ractor-safe.
Feel free to review more functions and mark them as safe later, I would just recommend to add them to test_ractor_parallel to check if they are indeed thread/Ractor-safe.

@eregon eregon merged commit ae62b76 into master Mar 25, 2025
42 checks passed
@eregon eregon deleted the not-ractor-safe branch March 25, 2025 20:16
@eregon
Copy link
Member Author

eregon commented Mar 25, 2025

I think methods from Etc.sysconfdir to Etc.nprocessors are ractor-safe.

Actually sysconfdir doesn't work in a Ractor: https://github.com/ruby/ruby/actions/runs/14069312270/job/39399502142#step:12:941 because it tries to access RbConfig::CONFIG if LOAD_RELATIVE.
So I'll just mark it as Ractor-unsafe then: #54
OTOH I'm happy to see --enable-load-relative is tested in CI.

@eregon
Copy link
Member Author

eregon commented Mar 27, 2025

Unfortunately there are more problems, which might be in Etc functions or Ractor bugs.
The test has been pended in ruby/ruby#12992

I have seen this failure:
https://github.com/ruby/ruby/actions/runs/14094646363/job/39479353266?pr=12984#step:14:814

  TestEtc#test_ractor_parallel [/Users/runner/work/ruby/ruby/src/test/etc/test_etc.rb:178]:
  assert_separately failed with error message
  pid 47346 exit 0
  | [BUG] pthread_mutex_lock: Invalid argument (EINVAL)
  | ruby 3.5.0dev (2025-03-26T22:25:25Z pull/12984/merge 1ea32181b7) +PRISM [arm64-darwin23]
  | 
  | -- Crash Report log information --------------------------------------------
  |    See Crash Report log file in one of the following locations:
  |      * ~/Library/Logs/DiagnosticReports
  |      * /Library/Logs/DiagnosticReports
  |    for more details.
  | Don't forget to include the above Crash Report log file in bug reports.
  | 
  | -- Control frame information -----------------------------------------------
  | 
  | 
  | -- Threading information ---------------------------------------------------
  | Total ractor count: 1
  | Ruby thread count for this ractor: 0
  | 
  | -- C level backtrace information -------------------------------------------
  | [BUG] Segmentation fault at 0xfffffffffffffff8
  | ruby 3.5.0dev (2025-03-26T22:25:25Z pull/12984/merge 1ea32181b7) +PRISM [arm64-darwin23]
  | 
  | -- Crash Report log information --------------------------------------------
  |    See Crash Report log file in one of the following locations:
  |      * ~/Library/Logs/DiagnosticReports
  |      * /Library/Logs/DiagnosticReports
  |    for more details.
  | Don't forget to include the above Crash Report log file in bug reports.
  | 
  | Crashed while printing bug report
  | [IMPORTANT]
  | Don't forget to include the Crash Report log file under
  | DiagnosticReports directory in bug reports.
  |

Mentioned in https://ruby.slack.com/archives/C02A3SL0S/p1743044607025709

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 this pull request may close these issues.

Etc is not Ractor-safe but is marked as such

3 participants