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

[BUGFIX] mutex like it should be #34

Closed
wants to merge 2 commits into from
Closed

[BUGFIX] mutex like it should be #34

wants to merge 2 commits into from

Conversation

genofire
Copy link
Member

@genofire genofire commented Mar 7, 2017

No description provided.

@genofire genofire requested a review from corny March 7, 2017 20:58
models/graph.go Outdated
builder.readNodes(nodes.List)
nodes.RUnlock()
Copy link
Member

Choose a reason for hiding this comment

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

Mach bessser builder.readNodes(nodes) und verschiebe die Locks in die Funktion.

models/graph.go Outdated
@@ -48,7 +48,9 @@ func (nodes *Nodes) BuildGraph() *Graph {
vpn: make(map[string]interface{}),
}

nodes.RLock()
Copy link
Member

Choose a reason for hiding this comment

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

Und das kann nicht mehr zu einem Deadlock führen?

Copy link
Member Author

@genofire genofire Mar 8, 2017

Choose a reason for hiding this comment

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

Ich hab keine Ahnung, wo der deadlock herkommt - doch dies soll mit diesen PR mit in Erfahrung gebracht werden.

Einmal alles untersuchen, was wo sich sperrt und ob es so richtig ist.

Copy link
Member

Choose a reason for hiding this comment

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

Vielleicht hiermit testen? https://github.com/sasha-s/go-deadlock

Copy link
Member Author

Choose a reason for hiding this comment

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

Hört sich interessant an :)

@corny
Copy link
Member

corny commented Mar 16, 2017

I don't think this should be merged into master.
If our code contains deadlocks, use this branch to detect them.

@genofire genofire added wip and removed review labels Mar 16, 2017
@genofire
Copy link
Member Author

greate, i will create a VM with a complete testing stack - for problemes like this.

@corny
Copy link
Member

corny commented Mar 16, 2017

You can also use the productive system as long as you monitor it and act in case of errors.

@coveralls
Copy link

coveralls commented May 22, 2017

Coverage Status

Coverage decreased (-0.08%) to 56.425% when pulling 4628a8d on improve-mutex into dc24c8b on master.

@corny
Copy link
Member

corny commented Oct 9, 2017

Liegt zu lange rum -> close.

@corny corny closed this Oct 9, 2017
@corny corny deleted the improve-mutex branch November 19, 2021 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants