Apparent race condition deleting objects results in ENOENT

[vlovich]

If I issue concurrent delete requests for objects that share a path, then the operation fails with a 500. Here's a sample typescript test using jest and the aws-sdk client library.

I think the root cause is that both are trying to do an rmdir up the chain of directories but they end up conflicting. Maybe rmdir should swallow ENOENT exceptions?

import { AWSError, S3 } from 'aws-sdk'
import S3rver from 's3rver'
import * as tmp from 'tmp'

function sync<T>(r: AWS.Request<T, AWS.AWSError>): Promise<T> {
  return new Promise((resolve, reject) => {
    r.send((err, data) => {
      if (err) {
        reject(err)
      } else {
        resolve(data)
      }
    })
  })
}

let s3: S3
let server: S3rver

beforeAll(() => {
    const serverDir = tmp.dirSync({ prefix: 's3-api.test', unsafeCleanup: true })

    server = new S3rver({
      port: 0,
      silent: false,
      directory: serverDir.name,
      configureBuckets: [
        {
          name: 'my-bucket',
          configs: [],
        },
      ],
    })
    const bound = await server.run()

    const s3 = new S3({
      region: 'us-east-1',
      accessKeyId: 'S3RVER',
      secretAccessKey: 'S3RVER',
      endpoint: `http://${bound.address}:${bound.port}`,
      s3ForcePathStyle: true,
    })
}

afterAll(async () => {
      await server.close()
})

test('s3rver racing delete', async ()  => {
    await Promise.all([
      sync(
        s3.putObject({
          Bucket: 'my-bucket',
          Key: '8ccf8a22d464800dccc55b90791fc6c43a867aeebda6d2772839f8a2105f9977/71f3db21645ab17a0f6fb561dad81608521e80a34d178646f738a98cb2ae8de4/0aa7509751f6c461ccd91c19e90360cccae3889ca83ecaa0415ff3bc0260f161/93bb309257da197aded081fa3de9c2c0',
        }),
      ),
      sync(
        s3.putObject({
          Bucket: 'my-bucket',
          Key: '8ccf8a22d464800dccc55b90791fc6c43a867aeebda6d2772839f8a2105f9977/71f3db21645ab17a0f6fb561dad81608521e80a34d178646f738a98cb2ae8de4/0aa7509751f6c461ccd91c19e90360cccae3889ca83ecaa0415ff3bc0260f161/93bb309257da197aded081fa3de9c2c0',
        }),
      ),
    ])

    await Promise.all([
      sync(
        s3.deleteObject({
          Bucket: 'my-bucket',
          Key: '8ccf8a22d464800dccc55b90791fc6c43a867aeebda6d2772839f8a2105f9977/71f3db21645ab17a0f6fb561dad81608521e80a34d178646f738a98cb2ae8de4/0aa7509751f6c461ccd91c19e90360cccae3889ca83ecaa0415ff3bc0260f161/93bb309257da197aded081fa3de9c2c0',
        }),
      ),
      sync(
        s3.deleteObject({
          Bucket: 'my-bucket',
          Key: '8ccf8a22d464800dccc55b90791fc6c43a867aeebda6d2772839f8a2105f9977/71f3db21645ab17a0f6fb561dad81608521e80a34d178646f738a98cb2ae8de4/0aa7509751f6c461ccd91c19e90360cccae3889ca83ecaa0415ff3bc0260f161/93bb309257da197aded081fa3de9c2c0',
        }),
      ),
    ])
}

[vlovich]

I think the fix is this:

     while (parts.length) {
       await fs.rmdir(path.join(bucketPath, ...parts)).catch(err => {
         if (err.code !== 'ENOENT' && err.code !== 'ENOTEMPTY') throw err;
         parts.length = 0;
       });
       parts.pop();
     }

This should also speed up deletion. Reasoning:

1. Failure to remove a component in the path (ENOENT) is not fatal - we can just assume something is racing us & move on. Deletion of the object has succeeded.
2. rmdir already returns an error (ENOTEMPTY) when the directory isn't empty. The readdirSync is superfluous & doesn't actually protect from race conditions.