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

fix DeadLock when it called unregisterConnector twice in same time. #2874

Closed
wants to merge 2 commits into from

Conversation

imtnd
Copy link

@imtnd imtnd commented Dec 29, 2019

fix a dead lock when called unregisterConnector twice in same time.

lock 1:
When socket connected:
NObserver::notify(Notification* pNf) <- lock
SocketConnector::onWritable(WritableNotification* pNotification)
SocketConnector::onConnect()
SocketConnector::unregisterConnector()
SocketReactor::removeEventHandler(const Socket& socket, const Poco::AbstractObserver& observer) <- wait to unlock

When delete SocketConnector:
SocketConnector::~SocketConnector()
SocketConnector::unregisterConnector() <- lock
SocketReactor::removeEventHandler(const Socket& socket, const Poco::AbstractObserver& observer)
SocketNotifier::removeObserver(SocketReactor* pReactor, const Poco::AbstractObserver& observer)
NotificationCenter::removeObserver(const AbstractObserver& observer)
NObserver::disable() <- wait to unlock

Test code maybe execute only on my machine.
I will remove test code if you don't need it.

@obiltschnig obiltschnig requested a review from aleks-f December 30, 2019 09:12
@obiltschnig
Copy link
Member

please re-submit against poco-1.10.0 branch, we do not accept PRs into master. Thank you!

@imtnd imtnd changed the base branch from master to poco-1.10.0 December 31, 2019 16:37
@imtnd imtnd changed the base branch from poco-1.10.0 to master December 31, 2019 16:40
@imtnd
Copy link
Author

imtnd commented Jan 1, 2020

I will re-submit against poco-1.10.0.
This PR closes.

@imtnd imtnd closed this Jan 1, 2020
@imtnd imtnd mentioned this pull request Jan 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants