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

HttpServerConnection#DataAvailableHandler(): be aware of being called multiple times concurrently #6817

Merged
merged 1 commit into from Dec 5, 2018

Conversation

Al2Klimov
Copy link
Member

@Al2Klimov Al2Klimov commented Dec 3, 2018

refs #6816

@Al2Klimov
Copy link
Member Author

This seems not to make #6815 obsolete in my test setup, but we could let the customer test it.

@dnsmichi dnsmichi added the area/api REST API label Dec 4, 2018
@dnsmichi
Copy link
Contributor

dnsmichi commented Dec 4, 2018

A similar problem might exist in the JsonRcpConnection class.

@Al2Klimov
Copy link
Member Author

No, JsonRpcConnection#m_DataHandlerMutex is locked only once, in JsonRpcConnection#DataAvailableHandler().

@dnsmichi
Copy link
Contributor

dnsmichi commented Dec 5, 2018

This can be called recursively from within Disconnect() -> TlsStream::Close() -> SignalDataAvailable(). The solution was a while ago to use a recursive mutex for HTTP handles.

@Al2Klimov
Copy link
Member Author

What about Disconnect() is just setting a flag and the data handler is called only in the socket thread?

@dnsmichi
Copy link
Contributor

dnsmichi commented Dec 5, 2018

We'll discuss this offline and focus on this fix.

@dnsmichi dnsmichi added the bug Something isn't working label Dec 5, 2018
@dnsmichi dnsmichi added this to the 2.10.3 milestone Dec 5, 2018
@dnsmichi dnsmichi merged commit b58ce84 into master Dec 5, 2018
@Al2Klimov Al2Klimov deleted the bugfix/stalled-tls-connections-6816 branch December 5, 2018 13:30
@dnsmichi dnsmichi added the backported Fix was included in a bugfix release label Dec 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api REST API backported Fix was included in a bugfix release bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants