Merge pull request #8982 from romayalon/romy-getpwnam-concurrency-issue

NSFS | FS Napi | getpwnam concurrency issue

Author: romayalon

src/native/fs/fs_napi.cpp

@@ -168,6 +168,7 @@ typedef std::map<std::string, std::string> XattrMap;
 const char* gpfs_dl_path = std::getenv("GPFS_DL_PATH");
 
 int gpfs_lib_file_exists = -1;
+static long PASSWD_BUF_SIZE = -1;
 
 static int (*dlsym_gpfs_fcntl)(gpfs_file_t file, void* arg) = 0;
 
@@ -1397,17 +1398,29 @@ struct Fsync : public FSWorker
 struct GetPwName : public FSWorker
 {
     std::string _user;
+    struct passwd _pwd;
     struct passwd *_getpwnam_res;
+    std::unique_ptr<char[]> _buf;
     GetPwName(const Napi::CallbackInfo& info)
         : FSWorker(info)
+        , _getpwnam_res(NULL)
     {
         _user = info[1].As<Napi::String>();
         Begin(XSTR() << "GetPwName " << DVAL(_user));
     }
     virtual void Work()
     {
-        _getpwnam_res = getpwnam(_user.c_str());
-        if (_getpwnam_res == NULL) SetSyscallError();
+        _buf.reset(new char[PASSWD_BUF_SIZE]);
+        if (!_buf) {
+            SetSyscallError();
+            return;
+        }
+        int rc = getpwnam_r(_user.c_str(), &_pwd, _buf.get(), PASSWD_BUF_SIZE, &_getpwnam_res);
+        if (rc != 0) {
+            SetSyscallError();
+            return;
+        }
+        if (_getpwnam_res == NULL) SetError("NO_SUCH_USER");
     }
 
     virtual void OnOK()
@@ -2402,6 +2415,12 @@ fs_napi(Napi::Env env, Napi::Object exports)
     exports_fs["DT_DIR"] = Napi::Number::New(env, DT_DIR);
     exports_fs["DT_LNK"] = Napi::Number::New(env, DT_LNK);
     exports_fs["PLATFORM_IOV_MAX"] = Napi::Number::New(env, IOV_MAX);
+    long passwd_bufsize = sysconf(_SC_GETPW_R_SIZE_MAX);
+    if (passwd_bufsize == -1) {
+        passwd_bufsize = 16384;
+    }
+    PASSWD_BUF_SIZE = passwd_bufsize;
+
 
 #ifdef O_DIRECT
     exports_fs["O_DIRECT"] = Napi::Number::New(env, O_DIRECT);
@@ -2415,6 +2434,7 @@ fs_napi(Napi::Env env, Napi::Object exports)
     exports_fs["set_log_config"] = Napi::Function::New(env, set_log_config);
 
     exports["fs"] = exports_fs;
+
 }
 
 } // namespace noobaa

src/test/unit_tests/jest_tests/test_fs_napi_concurrency.test.js

@@ -0,0 +1,55 @@
+/* Copyright (C) 2016 NooBaa */
+'use strict';
+
+const P = require('../../../util/promise');
+const nb_native = require('../../../util/nb_native');
+const test_utils = require('../../system_tests/test_utils');
+const { TEST_TIMEOUT } = require('../../system_tests/test_utils');
+const { get_process_fs_context } = require('../../../util/native_fs_utils');
+
+const DEFAULT_FS_CONFIG = get_process_fs_context();
+
+describe('fs napi concurrency tests', function() {
+
+    describe('getpwnam concurrency tests', function() {
+
+        const users = [];
+        beforeAll(async () => {
+            for (let i = 0; i < 20; i++) {
+                const username = `user-${i}`;
+                const expected_config = { uid: 7000 + i, gid: 7000 + i };
+                await test_utils.create_fs_user_by_platform(username, username, expected_config.uid, expected_config.gid);
+                users.push({ username, expected_config });
+            }
+        }, TEST_TIMEOUT);
+
+        afterAll(async () => {
+            for (let i = 0; i < users.length; i++) {
+                await test_utils.delete_fs_user_by_platform(users[i].username);
+            }
+        }, TEST_TIMEOUT);
+
+        it('getpwnam concurrency tests', async function() {
+            for (let i = 0; i < users.length; i++) {
+                nb_native().fs.getpwname(
+                    DEFAULT_FS_CONFIG,
+                    users[i].username
+                )
+                .catch(err => {
+                    console.log('getpwname error - ', err);
+                    throw err;
+                }).then(res => {
+                    console.log('getpwname res', res);
+                    users[i].actual_config = res;
+                });
+            }
+            await P.delay(5000);
+            for (let i = 0; i < users.length; i++) {
+                const actual = users[i].actual_config;
+                const expected = users[i].expected_config;
+                expect(actual.uid).toBe(expected.uid);
+                expect(actual.gid).toBe(expected.gid);
+            }
+            }, TEST_TIMEOUT);
+    });
+});

src/util/native_fs_utils.js

@@ -502,7 +502,7 @@ async function get_user_by_distinguished_name({ distinguished_name }) {
         const user = await nb_native().fs.getpwname(context, distinguished_name);
         return user;
     } catch (err) {
-        dbg.error('native_fs_utils.get_user_by_distinguished_name: failed with error', err, distinguished_name);
+        dbg.error('native_fs_utils.get_user_by_distinguished_name: failed with error', err, err.code, distinguished_name);
         if (err.code !== undefined) throw err;
         throw new RpcError('NO_SUCH_USER', 'User with distinguished_name not found', err);
     }