Skip to content
This repository has been archived by the owner on Feb 22, 2020. It is now read-only.

Issues/1319 shacl deadlock #183

Merged
merged 2 commits into from
Mar 6, 2019
Merged

Issues/1319 shacl deadlock #183

merged 2 commits into from
Mar 6, 2019

Conversation

hmottestad
Copy link
Contributor

This PR addresses GitHub issue: eclipse-rdf4j/rdf4j#1319 .

Briefly describe the changes proposed in this PR:

  • make a deadlock test
  • move super.prepare() and previousStateConnection.prepare() to final step in prepare()

…m locking until commit()

Signed-off-by: Håvard Ottestad <[email protected]>
Signed-off-by: Håvard Ottestad <[email protected]>
@hmottestad hmottestad requested a review from abrokenjester March 6, 2019 09:12
@hmottestad
Copy link
Contributor Author

hmottestad commented Mar 6, 2019

@jeenbroekstra I managed to find out what was causing the deadlock. At some point down the line of prepare() there is a semaphore.lock() without a corresponding unlock(). There is an unlock(), but I think it's called from somewhere else, as part of a close() process. Every once in a while we would then get a deadlock (and never when debugging).

Solution id to first validate all the data and then call super.prepare().

I'm not sure this fixes all cases of deadlock, but I haven't been able to produce any others.

Are you comfortable with pushing this into the 2.5 release, or do you want to wait for a 2.5.x or 3.0?

}
}finally {
super.prepare();
previousStateConnection.prepare();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

super.prepare();
previousStateConnection.prepare();

As the final step in prepare()

@abrokenjester
Copy link
Contributor

Looks good, great find! Let's merge this in.

@hmottestad hmottestad merged commit 313c972 into master Mar 6, 2019
@hmottestad hmottestad deleted the issues/1319_shacl_deadlock branch March 7, 2019 06:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants