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

Jump to bottom

Improve support for multiple JaCoCo runtimes in the same VM #1057

Merged
merged 10 commits into from Apr 4, 2022
Merged

Improve support for multiple JaCoCo runtimes in the same VM #1057

merged 10 commits into from Apr 4, 2022

Conversation

marchof
Copy link
Member

@marchof marchof commented Jun 8, 2020

FileChannel.lock() throws OverlappingFileLockException when the file is
locked from within the same JVM. This can happen when multiple JaCoCo
agents are loaded by different classloaders.

Fixes #331

@marchof marchof added this to the 0.8.6 milestone Jun 8, 2020
@marchof marchof self-assigned this Jun 8, 2020
@marchof
Copy link
Member Author

marchof commented Jun 9, 2020

In Java 5 OverlappingFileLockException is not thrown, therefore unit tests fail for Java 5.

https://www.oracle.com/technetwork/java/javase/compatibility-137541.html

@Godin Godin mentioned this pull request Jun 17, 2020
Closed
@famod
Copy link

famod commented Jun 17, 2020 •
edited

@marchof I've just tested this PR with my Quarkus reproducer and there is no OverlappingFileLockException anymore. 👍

cc @Godin

@famod famod mentioned this pull request Jun 17, 2020
Closed
@marchof marchof requested a review from Godin June 18, 2020 07:00
@marchof marchof added this to Implementation in Current work items via automation Jun 18, 2020
@marchof marchof moved this from Implementation to Review in Current work items Jun 18, 2020
@marchof marchof changed the title Retry exec file locking when file is locked from same JVM Improved support for multiple JaCoCo runtime in the same VM Jun 18, 2020
@marchof marchof changed the title Improved support for multiple JaCoCo runtime in the same VM Improved support for multiple JaCoCo runtimes in the same VM Jun 18, 2020
@j-be
Copy link

j-be commented Aug 8, 2020

@marchof many thanks for this.

For what its worth: I was hit by the issue using Quarkus 1.6.1 and can confirm your fix.

henrik242
henrik242 approved these changes Aug 31, 2020
View reviewed changes
Copy link

@henrik242 henrik242 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@henrik242 henrik242 mentioned this pull request Sep 15, 2020
Closed
@Godin Godin removed this from the 0.8.6 milestone Sep 15, 2020
@tisoft
Copy link

tisoft commented Oct 2, 2020

I am using this branch in my quarkus project now since a week and it fixes all(*) my issues.

Would be great if it would make it into 0.8.7.

(*) Well, all my coverage related issues, anyway.

jorsol
jorsol approved these changes Nov 18, 2020
View reviewed changes
Copy link

@jorsol jorsol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! confirmed the fix locally in a Quarkus application.

Please target this to 0.8.7

@pilhuhn
Copy link

pilhuhn commented Jan 14, 2021

Please include this in the next JaCoCo release, hopefully soon

@marchof
Copy link
Member Author

marchof commented Jun 29, 2021

Hi @Godin I try to pick up this PR again.

Should I cherry-pick the commit "Add org.jacoco.core.test to dependencyManagement" into a separate PR to make this PR smaller and more specific?

@marchof
Retry exec file locking when file is locked from same JVM
c350ef6 
FileChannel.lock() throws OverlappingFileLockException when the file is
locked from within the same JVM. This can happen when multiple JaCoCo
agents are loaded by different classloaders
@marchof
OverlappingFileLockException only thrown since Java 1.6
c74b012
@marchof
Consolidate place to check current JDK version
7a11b6b
@marchof
Update change log
bc94706
@Godin
Copy link
Member

Godin commented Jul 5, 2021

Should I cherry-pick the commit "Add org.jacoco.core.test to dependencyManagement" into a separate PR to make this PR smaller and more specific?

@marchof 👍 and done - 1c113ff

@Godin Godin added this to the 0.8.8 milestone Jul 5, 2021
marchof and others added 4 commits July 8, 2021 10:42
@marchof @Godin
Fix typo
6b4fa1b 
Co-authored-by: Evgeny Mandrikov <138671+Godin@users.noreply.github.com>
@marchof @Godin
Review: Fix type
dd60a52 
Co-authored-by: Evgeny Mandrikov <138671+Godin@users.noreply.github.com>
@marchof @Godin
Review: Fix typo
d07386c 
Co-authored-by: Evgeny Mandrikov <138671+Godin@users.noreply.github.com>
@marchof
Review comment: Adjust test method naming to actual test case
de55b08
@marchof
Copy link
Member Author

marchof commented Jul 8, 2021

@Godin Thx for the review! I applied your proposals.

@Godin Godin changed the title Improved support for multiple JaCoCo runtimes in the same VM Improve support for multiple JaCoCo runtimes in the same VM Apr 4, 2022
@Godin Godin merged commit aa16a7c into master Apr 4, 2022
Current work items automation moved this from Review to Done Apr 4, 2022
@Godin Godin deleted the retry-execfile-lock branch April 4, 2022 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@henrik242 henrik242 henrik242 approved these changes

@Godin Godin Godin approved these changes

@jorsol jorsol jorsol approved these changes

Assignees

@marchof marchof

Labels
component: core type: enhancement
Projects
Current work items
  
Done
Milestone
0.8.8
Development

Successfully merging this pull request may close these issues.

java.nio.channels.OverlappingFileLockException for Jacoco offline
8 participants
@marchof @famod @j-be @tisoft @pilhuhn @Godin @henrik242 @jorsol