search

browsermt / mts

Marian Translation Service
Apache License 2.0
18 stars 4 forks source link

Concurrency bug wrt to running_ #16

Closed kpu closed 3 years ago

kpu commented 3 years ago

What is the point of having running_ be atomic?

https://github.com/browsermt/mts/blob/d9aa0c50f56e91c83e53449ce197d337301af3df/src/bergamot/service.h#L31

It suggests that stop() might be called concurrently. However, this pattern does not correctly guard against reentry:

https://github.com/browsermt/mts/blob/d9aa0c50f56e91c83e53449ce197d337301af3df/src/bergamot/service.cpp#L75-L76

kpu commented 3 years ago

In this case we may as well assume Service's functions are not reentrant. In which case, get rid of running_ and add workers_.clear() at the bottom of Service::stop().