HotswapProjects / HotswapAgent

Java unlimited redefinition of classes at runtime.
GNU General Public License v2.0
2.37k stars 493 forks source link

Sometimes a hotswap is missed #614

Open Artur- opened 1 week ago

Artur- commented 1 week ago

We have a test that basically does 10-15 iterations with different cases using steps like:

  1. Modify source
  2. Compile source
  3. Wait for HotswapAgent to hotswap changes
  4. Verify changes

This test works about 50% of the time and the rest of the time it fails at one of the iterations (which exact one changes each time) but the test always fails because the step "3. Wait for HotswapAgent to hotswap changes" does not complete.

For a successful iteration, the hotswap agent logs look like https://gist.github.com/Artur-/914812e7622242cdb5b3fdb75e9c7a78 For a failed iteration, the hotswap agent logs look like https://gist.github.com/Artur-/03a87357331b395edbcb00abdc9b2407

The main difference seems to be that a resource change event is sent for the class but not a class redefined event, nor is the class ever hotswapped.

When this happens, if I do

touch target/classes/com/vaadin/copilottest/flow/views/helloworld/HelloWorldView.class

then HotswapAgent picks up the change, hotswaps, send plugin events etc as can be seen in https://gist.github.com/Artur-/8e33b01a0628893e2e7716e38ad0108d

So all changes are compiled but for some (timing related) reason, the class is not redefined

skybber commented 1 week ago

Thank you for reporting this issue. Could you confirm if you are using redefinition initiated from HA with autoHotSwap=true? Is there any specific reason for preferring autoHotSwap over the traditional debugger, which is generally more reliable? Otherwise it would be nice to supply some minimal project example to test it.

Artur- commented 1 week ago

This is with autoHotswap=true and run as a GitHub action job

skybber commented 1 week ago

Is it with the last HA version from repository?

Artur- commented 1 week ago

It is 2.0.1 - I will test with the latest

skybber commented 1 week ago

According comparison of both logs, the Watcher triggers the same sequence of modify/delete events:

HOTSWAP AGENT:  DEBUG (org.hotswap.agent.watch.nio.WatcherNIO2) - Watch event 'ENTRY_MODIFY' on '/projectfolder/target/classes/com/vaadin/copilottest/flow/views/helloworld/HelloWorldView.class' --> HelloWorldView.class
HOTSWAP AGENT:  DEBUG (org.hotswap.agent.watch.nio.WatcherNIO2) - Watcher on /projectfolder/target/classes/com/vaadin/copilottest/flow/views/helloworld not valid, removing path=
HOTSWAP AGENT:  DEBUG (org.hotswap.agent.watch.nio.WatcherNIO2) - Watch event 'ENTRY_DELETE' on '/projectfolder/target/classes/com/vaadin/copilottest/flow/views/helloworld' --> helloworld
HOTSWAP AGENT:  DEBUG (org.hotswap.agent.watch.nio.WatcherNIO2) - Watch event 'ENTRY_CREATE' on '/projectfolder/target/classes/com/vaadin/copilottest/flow/views/helloworld' --> helloworld
HOTSWAP AGENT:  DEBUG (org.hotswap.agent.watch.nio.WatcherNIO2) - Registering directory  /projectfolder/target/classes/com/vaadin/copilottest/flow/views/helloworld

According HA code, events are processed in WatchEventCommand.createCmdForEvent(), it seems to me, that in case when the redefinition is skipped, the event is lost at this place:

https://github.com/HotswapProjects/HotswapAgent/blob/master/hotswap-agent-core/src/main/java/org/hotswap/agent/annotation/handler/WatchEventCommand.java#L65

Code should check for existence of the file before this condition and log information that file is deleted at least. I will prepare the fix; maybe you can check this hypothesis in advance.

Artur- commented 1 week ago

Some more trace logging would be great to understand the problem better. I have found some other race conditions in our tests that I need to resolve also to ensure they are not causing this. It seems a bit like two compilations of the same file can happen in parallel sometimes, which might somehow explain this issue.

skybber commented 1 week ago
Artur- commented 1 week ago

I have been doing some more testing but the results are so far inconclusive at best... could you release a 2.0.2 version so it would be easier to test and use only one version?