Aquarium is not thread-safe

[paneq]

require 'aquarium'

class LoggingAspect
  include Aquarium::Aspects

  def initialize(logger)
    @logger = logger
  end

  def apply(auth_usecase)
    Aspect.new :after, methods: [:run], objects: [auth_usecase] do |jp|
      if !jp.context.raised_exception
        project = jp.context.returned_value
        @logger.info("Project authenticated: #{project}")
      else
        auth_key = jp.context.parameters[0]
        @logger.warn("Project failed to authenticate: #{auth_key}")
        @logger.warn("Error: #{jp.context.raised_exception}")
      end
    end
  end
end

class AuthUsecase
  def run(auth_key)
    auth_key
  end
end

a = AuthUsecase.new
l = Logger.new("output.log")

LoggingAspect.new(l).apply(a)

Thread.new do
  loop do
    result = a.run("123")
    msg = result == "123" ? "OK" : "WTF"
    l.info msg
  end
end

Thread.new do
  loop do
    result = a.run("ABC")
    msg = result == "ABC" ? "OK" : "WTF"
    l.info msg
  end
end

> grep "WTF" output.log | wc -l
20118
> grep "OK" output.log | wc -l
29891

Compare it to unaspected version:

class AuthUsecase
  def run(auth_key)
    auth_key
  end
end

a = AuthUsecase.new
l = Logger.new("output.log")

Thread.new do
  loop do
    result = a.run("123")
    msg = result == "123" ? "OK" : "WTF"
    l.info msg
  end
end

Thread.new do
  loop do
    result = a.run("ABC")
    msg = result == "ABC" ? "OK" : "WTF"
    l.info msg
  end
end

> grep "OK" output2.log | wc -l
113326
> grep "WTF" output2.log | wc -l
0

[andrzejkrzywda]

It can be related to this part of code:

https://github.com/deanwampler/Aquarium/blob/master/aquarium/lib/aquarium/aspects/aspect.rb#L435-L457

  #--
  # By NOT dup'ing the join_point, we save about 25% on the overhead! However, we
  # compromise thread safety, primarily because the join_point's context object will be changed.
  # TODO Refactor context out of static join point part.
  # Note that we have to assign the parameters and block to the context object in case
  # the advice calls "proceed" or "invoke_original_join_point" without arguments.
  #++

It seems to be an optimization, that may be less important in places where we care about Thread safety.

[deanwampler]

Thanks for pointing that out. I'll take a look this weekend. dean

On Fri, Jul 19, 2013 at 6:40 AM, Andrzej Krzywda notifications@github.comwrote:

It can be related to this part of code:

https://github.com/deanwampler/Aquarium/blob/master/aquarium/lib/aquarium/aspects/aspect.rb#L435-L457

--

By NOT dup'ing the join_point, we save about 25% on the overhead! However, we

compromise thread safety, primarily because the join_point's context object will be changed.

TODO Refactor context out of static join point part.

Note that we have to assign the parameters and block to the context object in case

the advice calls "proceed" or "invoke_original_join_point" without arguments.

++

It seems to be an optimization, that may be less important in places where we care about Thread safety.

— Reply to this email directly or view it on GitHubhttps://github.com/deanwampler/Aquarium/issues/39#issuecomment-21243017 .

Dean Wampler, Ph.D. "Functional Programming for Java Developers", "Programming Scala", and "Programming Hive" - all from O'Reilly twitter: @deanwampler, @chicagoscala http://polyglotprogramming.com

[andrzejkrzywda]

@deanwampler Any news here? We already use aquarium in places where thread-safety doesn't matter, but it would be great to use it also in thread-sensitive environments.

Is there any way we can help you here?

[chicagoscala]

Hi, Andrzej,

Thanks for bugging me about this. I haven't had any time recently to work on this, between client commitments and too many conferences recently. However, I took the time today and have it working.

It was actually a simple one-line change to Aspect::alias_original_method_text. It now makes a copy of the joinpoint rather than sharing a mutable one. I needed time to test this and verify that the performance was acceptable. When I wrote Aquarium circa 2006, the overhead for copies was 30%! It's now considerably smaller, no doubt due to improvements in both cruby and hardware.

I have pushed these changes to GitHub. It will be version 0.6.0, which I'll release as soon as I can. I need to get the jruby tests passing first and also update the packaging and publishing rake tasks (Unfortunately, since I don't work very often with Ruby these days, when I need to revisit Aquarium, I often need to update rake and other stuff, including the packaging and publishing tools, etc.)

So, I may not have time tomorrow or the next several days (travel), but worst case, I'll get that done by next weekend.

Yours, Dean

On Fri, Sep 27, 2013 at 8:22 AM, Andrzej Krzywda notifications@github.comwrote:

@deanwampler https://github.com/deanwampler Any news here? We already use aquarium in places where thread-safety doesn't matter, but it would be great to use it also in thread-sensitive environments.

Is there any way we can help you here?

—

Reply to this email directly or view it on GitHubhttps://github.com/deanwampler/Aquarium/issues/39#issuecomment-25244458 .

Dean Wampler, Ph.D. "Functional Programming for Java Developers", "Programming Scala", and "Programming Hive" - all from O'Reilly twitter: @deanwampler, @chicagoscala http://polyglotprogramming.com

[deanwampler]

Ij

[deanwampler]

I released v0.6.0 with this fix.