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

Getting new compile error on 1.3.0 #4840

Closed
myronmarston opened this issue Jun 21, 2016 · 13 comments
Closed

Getting new compile error on 1.3.0 #4840

myronmarston opened this issue Jun 21, 2016 · 13 comments

Comments

@myronmarston
Copy link
Contributor

Environment

  • Elixir version (elixir -v):
Erlang/OTP 18 [erts-7.2.1] [source] [64-bit] [smp:4:4] [async-threads:10] [hipe] [kernel-poll:false] [dtrace]

Elixir 1.3.0
  • Operating system:

OS X El Capitan

Current behavior

I'm getting a compile error when compiling my project from within Elixir's erlang source:

==> crawl_builder
Compiling 5 files (.ex)

== Compilation error on file lib/shaardwolf.ex ==
** (MatchError) no match of right hand side value: []
    (elixir) src/elixir_locals.erl:34: :elixir_locals.local_for/4

Here's the source code of the file that is failing (lib/shaardwolf.ex):

defmodule Delorean.CrawlBuilder.Shaardwolf do
  alias Delorean.CrawlBuilder.Shaardwolf.{ChunkSelector, ChunkStream}
  alias Delorean.Util.JSON

  defmodule Interface do
    @callback latest_page_issues_for(String.t) :: Stream.t
  end

  @behaviour Interface

  @moduledoc """
  Delorean.CrawlBuilder.Shaardwolf is the interface to Shaardwolf data. This is
  where we're expecting to get data for page_issues, site_issues, and pages, while
  avoiding the specifics of the internal structure relating to sorting and identifying
  the underlying s3 objects.
  """

  @doc """
  Gets a stream of crawl issues to build the current shard.
  """
  @spec latest_page_issues_for(String.t) :: Stream.t
  def latest_page_issues_for(campaign_id, injections \\ []) do
    chunk_stream_module = Keyword.get(injections, :chunk_stream_module, ChunkStream)
    chunk_id = %ChunkSelector{
      environment: Application.fetch_env!(:crawl_builder, :s3_environment),
      campaign_id: campaign_id,
      crawl_series_id: 1, # TODO: for now all crawls are for series 1
    }

    chunk_id
    |> chunk_stream_module.latest_chunks_for(:page_issues)
    |> unzip
    |> parse_jsonl
  end

  defp unzip(chunks) do
    Stream.map(chunks, &:zlib.gunzip/1)
  end

  defp parse_jsonl(detail_stream) do
    detail_stream
    |> Stream.flat_map(fn detail_chunk ->
      detail_chunk
      |> String.split("\n")
      |> Stream.map(&JSON.decode!/1)
    end)
  end
end

Note that this compile error consistently happens for me on OS X but I've not seen it happen in our travis builds (which are on linux) so there may be something OS-specific at play here.

Expected behavior

I'd expect it to compile this file without error. Or, if there is a problem with compiling this code, it should fail with a clear error explaining what the problem is instead of crashing with a MatchError in the compiler.

@myronmarston
Copy link
Contributor Author

Looking into this a bit on my own, it looks like the problem is on this line:

local_for(Module, Name, Arity) ->
  local_for(Module, Name, Arity, nil).
local_for(Module, Name, Arity, Given) ->
  Tuple = {Name, Arity},
  case elixir_def:lookup_clauses(Module, Tuple) of
    {Kind, Ann, [_ | _] = Clauses} when Given == nil; Kind == Given ->
      get_function(Ann, Module, Clauses);
    _ ->
      [_ | T] = erlang:get_stacktrace(),
      erlang:raise(error, undef, [{Module, Name, Arity, []} | T])
  end.

The [_ | T] = erlang:get_stacktrace(), line assumes erlang:get_stacktrace() returns a non-empty list, but the docs specifically call out the fact that it can:

If there has not been any exceptions in a process, the stacktrace is []. After a code change for the process, the stacktrace can also be reset to [].

So it looks like this function needs to be tolerant of getting an empty list?

@josevalim
Copy link
Member

It seems the bug has always been there but changes to the compiler is making you trigger it now. I will investigate, calling erlang:get_stacktrace() in there is wrong for all purposes.

@myronmarston
Copy link
Contributor Author

Thanks, @josevalim. If you figure out a work around we can use for this issue, please let me know. We were hoping to deploy 1.3.0 to prod today but are going to hold off until this is fixed since we can't use 1.3.0 in local dev while this is happening.

@josevalim
Copy link
Member

@myronmarston I have a fix.

diff --git a/lib/elixir/src/elixir_locals.erl b/lib/elixir/src/elixir_locals.erl
index 04d8715..f756a90 100644
--- a/lib/elixir/src/elixir_locals.erl
+++ b/lib/elixir/src/elixir_locals.erl
@@ -31,7 +31,7 @@ local_for(Module, Name, Arity, Given) ->
     {Kind, Ann, [_ | _] = Clauses} when Given == nil; Kind == Given ->
       get_function(Ann, Module, Clauses);
     _ ->
-      [_ | T] = erlang:get_stacktrace(),
+      {current_stacktrace, [_ | T]} = erlang:process_info(self(), current_stacktrace),
       erlang:raise(error, undef, [{Module, Name, Arity, []} | T])
   end.

as soon as tests pass I will push to both master and v1.3 branches.

@josevalim
Copy link
Member

josevalim commented Jun 21, 2016

As a local workaround, this should be enough:

alias Delorean.CrawlBuilder.Shaardwolf.{ChunkSelector, ChunkStream}
+require Delorean.CrawlBuilder.Shaardwolf.{ChunkSelector, ChunkStream}

This will make sure those modules are loaded upfront so we don't try to load structs dynamically (which is leading to the branch that fails).

@myronmarston
Copy link
Contributor Author

Sounds good, @josevalim! Will 1.3.1 be released with this fix soon? I'm comfortable building from source in the v1.3 branch (I've been doing that for a bit) but my teammates are probably less comfortable with that.

@myronmarston
Copy link
Contributor Author

As a local workaround, this should be enough:

Thanks! Giving that a try...

@josevalim
Copy link
Member

@myronmarston Sorry, you cannot remove the alias, you need both the alias and the require. :)

@josevalim
Copy link
Member

Btw, we had a test for this:

test "macro with undefined local" do
    assert_compile_fail UndefinedFunctionError,
      "function Kernel.ErrorsTest.MacroWithUndefinedLocal.unknown/1" <>
      " is undefined (function unknown/1 is not available)",
      '''
      defmodule Kernel.ErrorsTest.MacroWithUndefinedLocal do
        defmacrop bar, do: unknown(1)
        def baz, do: bar()
      end
      '''
  end

But it was not triggering that clause because unfortunately the stacktrace was filled.

josevalim pushed a commit that referenced this issue Jun 21, 2016
Signed-off-by: José Valim <jose.valim@plataformatec.com.br>
@myronmarston
Copy link
Contributor Author

I confirmed the workaround you provided fixes the issue for us. Thanks! Now we don't have to wait until 1.3.1 to upgrade :).

@josevalim
Copy link
Member

@myronmarston and I can wait a bit more before releasing v1.3.1. :D

@myronmarston
Copy link
Contributor Author

We're running 1.3.0 in prod now :).

@josevalim
Copy link
Member

Yay, congrats and thanks for all of the feedback on the prereleases!

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

No branches or pull requests

2 participants