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

Concurrent::Map #inspect test error #547

Closed
voxik opened this issue Jul 4, 2016 · 9 comments
Closed

Concurrent::Map #inspect test error #547

voxik opened this issue Jul 4, 2016 · 9 comments
Labels
bug A bug in the library or documentation.

Comments

@voxik
Copy link
Contributor

voxik commented Jul 4, 2016

Trying to build package for Fedora ([1] but this link will expire quite soon), I got following error:

  1) Concurrent::Map #inspect
     Failure/Error: expect(Concurrent::Map.new.inspect).to match(regexp)
       expected "#<Concurrent::Map:0x-7db1e810 entries=0 default_proc=nil>" to match /\A#<Concurrent::Map:0x[0-9a-f]+ entries=[0-9]+ default_proc=.*>\Z/i
       Diff:
       @@ -1,2 +1,2 @@
       -/\A#<Concurrent::Map:0x[0-9a-f]+ entries=[0-9]+ default_proc=.*>\Z/i
       +"#<Concurrent::Map:0x-7db1e810 entries=0 default_proc=nil>"
     # ./spec/concurrent/map_spec.rb:831:in `block (2 levels) in <module:Concurrent>'

Please note this was 32b ARM builder, so is this some wrong mapping of integers in inspect method?

[1] http://koji.fedoraproject.org/koji/taskinfo?taskID=14762876

@jdantonio
Copy link
Member

Thanks for pointing this out. I'll update the test with a better regex (or possibly remove the test--I'm not sure it's providing much value).

@voxik
Copy link
Contributor Author

voxik commented Jul 5, 2016

But I'd say that the test is pointing out an error in implementation of #inspect method (at minimun), since the "0x-7db1e810" does not look healthy IMO.

@pitr-ch pitr-ch added the bug A bug in the library or documentation. label Jul 29, 2016
@pitr-ch pitr-ch added this to the 1.0.3 milestone Jul 29, 2016
@voxik
Copy link
Contributor Author

voxik commented Oct 11, 2016

Interesting, today I met very similarly looking issue in Action View test suite:

  1) Failure:
ResolverCacheTest#test_inspect_shields_cache_internals [/builddir/build/BUILD/rubygem-actionview-5.0.0.1/usr/share/gems/gems/actionview-5.0.0.1/test/template/resolver_cache_test.rb:5]:
Expected /#<ActionView::Resolver:0x[0-9a-f]+ @cache=#<ActionView::Resolver::Cache:0x[0-9a-f]+ keys=0 queries=0>>/ to match "#<ActionView::Resolver:0x818727a8 @cache=#<ActionView::Resolver::Cache:0x-7e78d8a0 keys=0 queries=0>>".
1764 runs, 4042 assertions, 1 failures, 0 errors, 0 skips

Please note the ActionView::Resolver::Cache:0x-7e78d8a0. I am wondering if there is common source of this issues.

@pitr-ch pitr-ch modified the milestones: 1.0.4, 1.0.3 Dec 11, 2016
@pitr-ch pitr-ch modified the milestones: 1.0.4, 1.0.5 Dec 27, 2016
@pitr-ch pitr-ch modified the milestones: 1.0.5, 1.0.6 Feb 26, 2017
@pitr-ch pitr-ch modified the milestones: 1.0.6, 1.1.0 Mar 13, 2017
@pitr-ch pitr-ch modified the milestones: 1.1.1, 1.1.0 Apr 2, 2017
@pitr-ch
Copy link
Member

pitr-ch commented Apr 2, 2017

@voxik hi sorry for not addressing this sooner. I could not find any to_s or inspect implementation in our code. So I am inclined to update the regexp. Could you verify that -7e78d8a0 is indeed an allowed value on ARM 32 ?

@voxik
Copy link
Contributor Author

voxik commented Apr 3, 2017

So this error is obviously coming from [1]. And it seems to be signed/unsigned mismatch. E.g. something like this:

pry(main)> ([0x7fffffff].pack("l").unpack("l").first).to_s 16
=> "7fffffff"

pry(main)> ([0x80000000].pack("l").unpack("l").first).to_s 16
=> "-80000000"

But in reality, this should be unpacked as unsigned:

pry(main)> ([0x80000000].pack("l").unpack("L").first).to_s 16
=> "80000000"

Obviously, this is not issue for default Object#inspect, since it is using C sprintf function with pointer format [2].

@voxik
Copy link
Contributor Author

voxik commented Apr 3, 2017

This should be way to reproduce (be careful with the memory consumption!):

$ ruby << \EOR
GC.disable
class A
  DEFAULT_OBJ_ID_STR_WIDTH = 0.size == 4 ? 7 : 14

  def inspect
    id_str = (object_id << 1).to_s(16).rjust(DEFAULT_OBJ_ID_STR_WIDTH, '0') 
    "#<#{self.class.name}:0x#{id_str}>"
  end
end
3_000_000.times { p A.new.inspect }
EOR

"#<A:0x58585428>"
"#<A:0x585852d4>"
"#<A:0x585851bc>"
"#<A:0x5858507c>"
"#<A:0x58584ec4>"
"#<A:0x58584d5c>"
"#<A:0x58584c1c>"
"#<A:0x58584adc>"

... snip ...

"#<A:0x7fff4888>"
"#<A:0x7fff47c0>"
"#<A:0x7fff46f8>"
"#<A:0x7fff4630>"
"#<A:0x7fff4568>"
"#<A:0x7fff44a0>"
"#<A:0x7fff43d8>"
"#<A:0x7fff4310>"
"#<A:0x7fff4248>"
"#<A:0x7fff4180>"
"#<A:0x7fff40b8>"
"#<A:0x-7fffc034>"
"#<A:0x-7fffc110>"
"#<A:0x-7fffc1ec>"
"#<A:0x-7fffc2c8>"
"#<A:0x-7fffc3a4>"
"#<A:0x-7fffc480>"
"#<A:0x-7fffc55c>"
"#<A:0x-7fffc638>"
^C-:10:in `p': Interrupt
	from -:10:in `block in <main>'
	from -:10:in `times'
	from -:10:in `<main>'

When the address crosses certain level, it becomes negative. Trying this with:

$ ruby -v
ruby 2.3.3p222 (2016-11-21 revision 56859) [i386-linux]

@voxik
Copy link
Contributor Author

voxik commented Apr 3, 2017

On the other hand, object_id appears to be signed value (see the source at [1]), which doesn't make a whole lot sense to me

@voxik
Copy link
Contributor Author

voxik commented Apr 3, 2017

I reported the issue to Ruby upstream:

https://bugs.ruby-lang.org/issues/13397

@pitr-ch pitr-ch modified the milestones: 1.1.1, 1.1.0 Feb 21, 2018
@pitr-ch
Copy link
Member

pitr-ch commented Feb 25, 2018

I think this is fixed in #699 or #651

@pitr-ch pitr-ch closed this as completed Feb 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug in the library or documentation.
Projects
None yet
Development

No branches or pull requests

3 participants