padrino-framework icon indicating copy to clipboard operation
padrino-framework copied to clipboard

Fix remove_constant to stop removing constants that are already remove_constanted but not GCed

Open genkami opened this issue 5 years ago • 2 comments

I found a bug with remove_constant and GC.

I wll later give an example that potentially causes this bug.

genkami avatar Nov 07 '18 23:11 genkami

@genkami Could you provide an example for reproducing the error?

namusyaka avatar Apr 11 '19 00:04 namusyaka

I added a test case that reproduces the errror. It fails without 8542c62.

This is how this error occurs:

  • First, we require two files: t.rb, u.rb
  • t.rb requires v.rb, w.rb, x.rb with nested require_dependencies.
  • The first loading of v.rb fails and V is removed because it depends on w.rb. (we call this V V1)
  • Then, w.rb is loaded successfully, and it defines W.
  • Then, we try to load x.rb, but it fails because x.rb depends on y.rb
  • Then, we try to load v.rb again, and it succeeds. (we call the V defined here V2)
  • Eventually, we find that we can't load x.rb because we can't resolve its dependencies.
  • After all, the entire process of loading t.rb is considered as failure.
  • All constants that are defined in loading a.rb are going to be rollbacked, except for V2 and W. In this situation, V1 are considered as a constant that should be removed because it's defined in neither v.rb nor w.rb. So we tries to remove V1, but it results to removing V2 because V1.to_s == V2.to_s == "V".
  • After rollback, we load x.rb and its dependency (y.rb).
  • Then, we try to load t.rb again, but only x.rb are loaded as t.rb's dependencies, because we already loaded v.rb with no rollback.
  • As a result, V is never defined.

genkami avatar Jul 12 '19 03:07 genkami