New filesystem plugins throw segfault

[vnghia]

It seems that the new filesystem plugins aren't stable. In fact, I just did some experiements and here is the result:

gfile.makedirs:

• s3: SEGFAULT
• gcs : SEGFAULT

io.read_file:

• s3 : OK
• gcs : OK

gfile.exists:

• s3: OK
• gcs: ABORT

I could not test everything right now, will update when I finish testing.

[yongtang]

@vnvo2409 Is this on Windows or Linux?

[vnghia]

@yongtang

I am testing on MacOS right now. But I think on Linux it will be the same.

In fact, I did see the same thing as you said in #1218 (comment). Maybe the filesystem plugins registration does not work correctly.

[vnghia]

@yongtang

I found the problem. In ModularFileSystem::RecursivelyCreateDir, we call FileSystem::RecursivelyCreateDir(dirname). And RecursivelyCreateDir(const std::string& dirname) will call the actual fallback with token return RecursivelyCreateDir(dirname, nullptr);. And instead of calling Filesystem:: RecursivelyCreateDir, it calls ModularFileSystem::RecursivelyCreateDir ( because we are still inside the ModularFileSystem class) which creates an infinite loop.

The fix is return FileSystem::RecursivelyCreateDir(dirname, nullptr); inside filesystem.h or return FileSystem::RecursivelyCreateDir(dirname, token); inside modular_filesystem.cc. Note that we have to fix these kinds of problem for all functions which use the default fallback.

I am not sure which one should be prefered. Do you have any suggestions ? @mihaimaruseac @yongtang

One more questions, could we make it into 2.4 ? @mihaimaruseac . Since all filesystems plugins ( including posix ) are broken.

[mihaimaruseac]

It is too late for 2.4 as it will be released today.

This is a side effect of the transaction tokens. We should fix modular_filesystem.cc to call using the token argument.

Then, we can make a patch on the 2.4 branch and fix in an upcoming patch release, though I'm unsure of the timeline for that.