You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2021/07/15 09:27:52 UTC
[GitHub] [celix] jenny-cheung opened a new issue #352: Potential errors due to the incorrect lock usage
jenny-cheung opened a new issue #352:
URL: https://github.com/apache/celix/issues/352
Dear Developers:
First, thank you for your checking! I found a potential error in the below code. Should the lock mutex be locked and then unlocked? Potential errors such as data race, deadlock may be triggered.
https://github.com/apache/celix/blob/0729c4dfaa6b45f436e41f13ebe71960c746d109/bundles/remote_services/civetweb/src/civetweb.c#L1762
```
static int pthread_cond_timedwait(pthread_cond_t *cv, pthread_mutex_t *mutex, const struct timespec * abstime)
{
...;
pthread_mutex_unlock(mutex); // here
ok = (WAIT_OBJECT_0 == WaitForSingleObject(tls->pthread_cond_helper_mutex, mswaitrel));
pthread_mutex_lock(mutex); //here
return ok ? 0 : -1;
}
```
Best,
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] jenny-cheung commented on issue #352: Potential errors due to the incorrect lock usage
Posted by GitBox <gi...@apache.org>.
jenny-cheung commented on issue #352:
URL: https://github.com/apache/celix/issues/352#issuecomment-885523833
Hi, developers @pnoltes
Any comments would be highly appreciated. Thank you.
Best Regards,
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] jenny-cheung commented on issue #352: Potential errors due to the incorrect lock usage
Posted by GitBox <gi...@apache.org>.
jenny-cheung commented on issue #352:
URL: https://github.com/apache/celix/issues/352#issuecomment-889929152
@pnoltes Thank you for your detailed reply! Let me propose an issue at civetweb. I will let you know if there is any progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes edited a comment on issue #352: Potential errors due to the incorrect lock usage
Posted by GitBox <gi...@apache.org>.
pnoltes edited a comment on issue #352:
URL: https://github.com/apache/celix/issues/352#issuecomment-889862814
unlocking and directly locking is indeed strange and for me a code smell.
That said, this is civetweb code. Celix embeds civetweb for remote service and the http admin:
https://github.com/apache/celix/tree/0729c4dfaa6b45f436e41f13ebe71960c746d109/bundles/remote_services/civetweb/src
https://github.com/apache/celix/tree/0729c4dfaa6b45f436e41f13ebe71960c746d109/bundles/http_admin/civetweb
Ideally the remote service admin should use the http admin instead of reallying directly on civetweb, but because remote service predates the http admin bundle this has not been done.
Celix does not maintain the civetweb code. The civetweb project can be found at
https://github.com/civetweb/civetweb
Looking at the code at civetweb, the approach has changed, but the unlock then lock idiom is still there:
https://github.com/civetweb/civetweb/blob/master/src/civetweb.c#L4785-L4807
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] jenny-cheung commented on issue #352: Potential errors due to the incorrect lock usage
Posted by GitBox <gi...@apache.org>.
jenny-cheung commented on issue #352:
URL: https://github.com/apache/celix/issues/352#issuecomment-890486344
@pnoltes Hi, I forgot to check the function name and made a mistake. This is in the[ pthread_cond_timedwait](https://pubs.opengroup.org/onlinepubs/009604599/functions/pthread_cond_timedwait.html) method.
> These functions atomically release mutex and cause the calling thread to block on the condition variable cond; atomically here means "atomically with respect to access by another thread to the mutex and then the condition variable".
> That is if another thread is able to acquire the mutex after the about-to-block thread has released it, then a subsequent call to pthread_cond_broadcast() or pthread_cond_signal() in that thread shall behave as if it were issued after the about-to-block thread has blocked.
> Upon successful return, the mutex shall have been locked and shall be owned by the calling thread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on issue #352: Potential errors due to the incorrect lock usage
Posted by GitBox <gi...@apache.org>.
pnoltes commented on issue #352:
URL: https://github.com/apache/celix/issues/352#issuecomment-889862814
unlocking and directly locking is indeed strange and for me a code smell.
That said, this is civetweb code. Celix embeds civetweb for remote service and the http admin:
https://github.com/apache/celix/tree/0729c4dfaa6b45f436e41f13ebe71960c746d109/bundles/remote_services/civetweb/src
https://github.com/apache/celix/tree/0729c4dfaa6b45f436e41f13ebe71960c746d109/bundles/http_admin/civetweb
Ideally the remote service admin should use the http admin instead of reallying directly on civetweb, but because remote service predates the http admin bundle this has not been done.
Celix does notmaintain the civetweb code. The civetweb project can be found at
https://github.com/civetweb/civetweb
Looking at the code at civetweb, the approach has changed, but the unlock then lock idiom is still there:
https://github.com/civetweb/civetweb/blob/master/src/civetweb.c#L4785-L4807
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] jenny-cheung closed issue #352: Potential errors due to the incorrect lock usage
Posted by GitBox <gi...@apache.org>.
jenny-cheung closed issue #352:
URL: https://github.com/apache/celix/issues/352
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] jenny-cheung commented on issue #352: Potential errors due to the incorrect lock usage
Posted by GitBox <gi...@apache.org>.
jenny-cheung commented on issue #352:
URL: https://github.com/apache/celix/issues/352#issuecomment-885569150
@pnoltes OK. Have a good vacation!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on issue #352: Potential errors due to the incorrect lock usage
Posted by GitBox <gi...@apache.org>.
pnoltes commented on issue #352:
URL: https://github.com/apache/celix/issues/352#issuecomment-885548106
Hi, I am on vacation for now. I will try and have a look at this next week.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org