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

Logger: log rotation inter-process lock failed. on Windows with JRuby 9.x #4147

Closed
smk0621 opened this Issue Sep 12, 2016 · 1 comment

Comments

Projects
None yet
2 participants
@smk0621
Contributor

smk0621 commented Sep 12, 2016

Environment

Provide at least:

  • jruby 9.1.2.0 (2.3.0) 2016-05-26 7357c8f Java HotSpot(TM) 64-Bit Server VM 25.102-b14 on 1.8.0_102-b14 +jit [mswin32-x86_64]
  • Windows 10 + Oracle Java SE JDK 1.8.0_102

Test Codes

  require 'logger'
  logger = Logger.new("C:\\log.txt", 5, 1024)
  150.times do |i|
    logger.info i
  end

Expected Behavior

  • Create "log.txt", "log.txt.0", "log.txt.1", "log.txt.2", "log.txt.3" files.

Actual Behavior

  • Raise error:
  log rotation inter-process lock failed. Permission denied - C:\log.txt or C:\log.txt.0
  log writing failed. closed stream
  log shifting failed. closed stream
  ...

We test it in JRuby1.6.8, JRuby1.7.22 and Ruby 2.3.1,
they are correct.

And we find the source codes in jruby-9.1.2.0\lib\ruby\stdlib\logger.rb line 745

  if /mswin|mingw/ =~ RUBY_PLATFORM
    def lock_shift_log
      yield
    end
  else
    ...
  end

For Ruby, the value of RUBY_PLATFORM likes "x64-mingw32".
But for JRuby, the value of RUBY_PLATFORM is "java".

For test, we change
if /mswin|mingw/ =~ RUBY_PLATFORM
to
if /mswin|mingw|java/ =~ RUBY_PLATFORM
it is correct too.

So, is this a JRuby bug?

@smk0621 smk0621 changed the title from Looger: log rotation inter-process lock failed. on Windows with JRuby 9.x to Logger: log rotation inter-process lock failed. on Windows with JRuby 9.x Sep 13, 2016

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Sep 13, 2016

Member

@smk0621 Ahh yes, a platform check. I think the proper fix for this would be to modify the check to use RbConfig rather than RUBY_PLATFORM, probably something like this:

if /mswin|mingw/ =~ RbConfig::CONFIG['host_os']

This will work on all Ruby versions.

Wanna test this and put together a PR for us?

This is the sort of thing we're generally comfortable patching in our copy of MRI's stdlib, but we do usually try to get such changes upstreamed to MRI. We'll handle that at some point.

Member

headius commented Sep 13, 2016

@smk0621 Ahh yes, a platform check. I think the proper fix for this would be to modify the check to use RbConfig rather than RUBY_PLATFORM, probably something like this:

if /mswin|mingw/ =~ RbConfig::CONFIG['host_os']

This will work on all Ruby versions.

Wanna test this and put together a PR for us?

This is the sort of thing we're generally comfortable patching in our copy of MRI's stdlib, but we do usually try to get such changes upstreamed to MRI. We'll handle that at some point.

@headius headius added this to the JRuby 9.1.6.0 milestone Sep 13, 2016

@smk0621 smk0621 referenced this issue Sep 14, 2016

Merged

Fix for #4147 #4153

@kares kares closed this in #4153 Sep 14, 2016

kares added a commit that referenced this issue Sep 14, 2016

headius added a commit to jruby/ruby that referenced this issue Nov 14, 2016

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