michalmuskala / jason

A blazing fast JSON parser and generator in pure Elixir.
Other
1.6k stars 170 forks source link

Test failing on otp 22 #104

Closed lukaszsamson closed 4 years ago

lukaszsamson commented 4 years ago

OTP version: Erlang/OTP 22 [erts-10.6.4] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [hipe] Elixir version: 1.8, 1.9, 1.10 Jason version: master@70b046aee882ca789bebf3a7d173e6259f3fa8aa

The following tests fails.

  1) test copying strings on decode (Jason.DecodeTest)
     test/decode_test.exs:88
     Assertion with == failed
     code:  assert :binary.referenced_byte_size(key) == 14
     left:  3
     right: 14
     stacktrace:
       test/decode_test.exs:102: (test)

It looks like OTP 22 introduced some change in binary refcounting.

Dreamer009 commented 4 years ago

Looking though the Erlang repository the only thing that changed recently in binary.erl is:

https://github.com/erlang/otp/commit/08311d277b5e65b5442c863f02eb1a3f92491357#diff-3c70062d5e1bd00aa38ef8df8adf5c51

Since we are dealing with strings (character lists) is it possible that we no longer need to decide between:

defp string_decode_function(%{strings: :copy}), do: &:binary.copy/1 defp string_decode_function(%{strings: :reference}), do: &(&1)

I would be willing to write a merge request, or help contribute. Any thoughts from the maintainers?

michalmuskala commented 4 years ago

This possibly comes from some changes in the GC. This test is generally very implementation-dependent, but I don't really have an idea how to test it otherwise. The differentiation is probably still needed, though, I wouldn't trust the default behaviour without explicit :binary.copy/1 calls. It's possible we just need to operate on bigger strings in the test instead of just 4 characters.