search

LLNL / UnifyFS

UnifyFS: A file system for burst buffers
Other
106 stars 31 forks source link

Missing locking on sm->time_to_exit #700

Closed ryancaicse closed 2 years ago

ryancaicse commented 2 years ago

Hi, developers, lock sm->thrd_lock is used to protect sm->time_to_exit. However, it only protects the write sm->time_to_exit = 1; not the read if (sm->time_to_exit). For mutually exclusive accesses, the locks should be guarding both places.

https://github.com/LLNL/UnifyFS/blob/72808e11e821c9b02fc11014efb467a26b9eb18a/server/src/unifyfs_service_manager.c#L1478-L1480

https://github.com/LLNL/UnifyFS/blob/72808e11e821c9b02fc11014efb467a26b9eb18a/server/src/unifyfs_service_manager.c#L252-L255

MichaelBrim commented 2 years ago

This unlocked access to sm->time_to_exit is by design. The variable is initialized to 0, and once it is set to 1, the value will never change. It does not matter for correctness which loop iteration we detect the "time to exit" condition, only that we will eventually. The time window for "eventually" is around 50ms, which is plenty fast for this scenario when we are shutting down the server.

ryancaicse commented 2 years ago

@MichaelBrim Thanks so much for your explanations.