Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] TPacker.Do7zip: Command-line length limitation in Windows #1890

Open
3 tasks done
Stabilostick opened this issue Dec 14, 2024 · 13 comments
Open
3 tasks done

[bug] TPacker.Do7zip: Command-line length limitation in Windows #1890

Stabilostick opened this issue Dec 14, 2024 · 13 comments

Comments

@Stabilostick
Copy link

Stabilostick commented Dec 14, 2024

Prerequisites

  • I'm using the latest version.
  • All website modules are up to date.
  • This is not a website module issue.

System information

Windows 11 all versions

Describe the bug

When a command-line command in Windows exceeds a certain length, it fails to execute. This issue occurs in the application FMD2, specifically within the TPacker.Do7zip method, which invokes the 7za.exe executable to compress multiple files into a single .cbz archive.

To Reproduce

When attempting to process several hundred image files, the application throws the following error:

ERROR: TPacker.Do7zip Error: Unknown \\...\fmd2\...\7za.exe a -tzip -mx0 -mmt20 -sccUTF-8 -scsUTF-8 -stl -spd -slt -sse -sdel "<lots of files> <lots of files> <...>"

The error occurs consistently when the command-line string exceeds a certain length, specifically when a large number of file paths are passed to the 7za.exe executable. This indicates that the command-line length limit in Windows is being exceeded.

A typical source for reproduction is "+99 Reinforced Wooden Stick," which has chapters that can contain more than 350 files.

Expected behavior

To resolve this limitation, the implementation of TPacker.Do7zip in FMD2 should be adjusted. A recommended approach could be:

  • Use a Control File: Create a text file containing the fully qualified paths (FQDNs) of all image files to be included in the archive.
  • Pass the Control File to 7-zip: Use the @FileList parameter supported by 7za.exe to read the file paths from the control file instead of appending them directly to the command-line string.
  • 7za.exe a -tzip -mx0 -mmt20 -sccUTF-8 -scsUTF-8 -stl -spd -slt -sse -sdel "@filelist.txt" output.cbz

Alternative Approaches

  • Batch Processing: Split the list of files into smaller chunks that stay within the command-line length limit and process them in smaller batches.
  • Dynamic Detection: Implement logic in TPacker.Do7zip to detect when the command-line string approaches the length limit and adjust the number of files in the batch accordingly.
  • Use Libraries Instead of Executables: Replace 7za.exe with a library-based archiving solution (e.g., a 7-zip library) that does not rely on command-line execution, thereby bypassing this limitation.
  • ...

Screenshots

No response

Additional context

The maximum command-line length in Windows is known to be limited to approximately 32,768 characters. When this limit is surpassed, the operating system is unable to process the command, resulting in execution failure.

@Slasar41
Copy link
Collaborator

Should be fixed with this: 7aefc87
Here's the compiled program:
fmd.zip

@Stabilostick
Copy link
Author

Hmm. fmd.exe could not be started. Application error 0xc000007b.
This is because I use the 32bit-dlls. The 64bit version has more memory problems when handling large collections (or is the old lua54 64bit memory problem already fixed?). Could you please generate a 32bit version for testing?

@Stabilostick
Copy link
Author

Stabilostick commented Dec 14, 2024

I changed FMD2 completely to 64bit for testing. Your solution is working. But it is a little bit of a hack :-)

For example the dynamic creation and deletion of the temp folder for each run in the program folder. While functional, this approach has notable issues:

  • No error handling: Functions like CreateDir and RenameFile lack error checks here. Failures due to permission issues or other file system constraints are not accounted for.
  • Potential program directory issues: Using the program directory (.\temp) for temporary files may lead to permission issues or conflicts, especially on windows where writing to the program directory might be restricted.
  • Temporary folder management: Failing to clean up temporary folders after execution can lead to clutter and wasted storage.

In my case the FMD2 program folder is stored on a network share with snaphots on -> lots of clutter here.

You address path length issues by truncating paths or prepending the \?\ prefix to enable long path support. While this works, there are concerns. Simply cutting off paths at MAX_PATHDIR risks data loss and file mismanagement.

More elegant solution: Validate the path's length and provide meaningful feedback or alternative handling for overly long paths.

if Length(InputPath) > MAX_PATH then
begin
if not MainForm.cbOptionEnableLongNamePaths.Checked then
Logger.SendError(Self.ClassName+'.Do7Zip Error: Path exceeds maximum allowed length and long path support is disabled.');
-- return error Result
if Pos('\\?\', InputPath) = 0 then
Result := '\\?\'+ InputPath
else
Result := InputPath;
end
else
Result := InputPath;

But now the most important thing:
"THANK YOU!!!". You don’t even get a solution this quickly with professional support contracts. A heartfelt thank you for your work and, above all, for always staying on it

@Slasar41
Copy link
Collaborator

This is because I use the 32bit-dlls. The 64bit version has more memory problems when handling large collections (or is the old lua54 64bit memory problem already fixed?).

Now that you use 64bit. Does memory problems still happen?

Thanks for the feedback. I'll let the person who made the change know when they're available.

@Stabilostick
Copy link
Author

Does memory problems still happen?

I bet 'yes'.

See attached screenshot. Shows 45min runtime loading 150 download tasks with approx. 50 chapters each slowly. 45.000 tasks in the "all" list. Memoy consumption increased in time of no active data transfer too (see beginning of graph with no network io activity).

Just a guess: might be GC from lua and memory fragmentation? Mem-start is around 700MB, end is 1.7GB and still 70 download tasks to go.

@Stabilostick
Copy link
Author

Screenshot 2024-12-15 021557

@Slasar41
Copy link
Collaborator

I've compiled 32bit version. Could you test it too?
fmd.zip

@Stabilostick
Copy link
Author

Stabilostick commented Dec 15, 2024

Additional info for 64bit. Memory after downloads fnished
grafik
-> Heap

Increase of different memory alloc types over time:
grafik

Waited 10min for GC (or a wonder) :-)
grafik
-> Nothing happened. No memory release. The green block for CPU usage is the manual execution of the "merge downloads" function.

Next info will be about 32bit.

@Stabilostick
Copy link
Author

(Perhaps we should transfer this to a new issue?)

@Stabilostick
Copy link
Author

Back to the original issue:

As suspected, after around 200 download tasks, there's a lot of junk in the temp folder.
grafik

So it's working, But this is probably not an elegant solution.
It would be probably better to generate a temporary text file for each chapter with the FQDNs of all files of a folder and use this file as a parameter file for 7za.exe

@Stabilostick
Copy link
Author

32bit version is not working at all (new parameter "long path names" is enabled)

03:39:52:190 ERROR: Failed to create dir(88) = \\?\\\<nasname>\<sharename>\<folder>\<folder>\<site>\<name>\Chapter 032.1\
... and so on.

@Slasar41
Copy link
Collaborator

You can create a new issue regarding memory problems.

32bit version is not working at all (new parameter "long path names" is enabled)

Glad it is not released yet. I don't test much with 32bit version.

Do you now Pascal?
Maybe you can try help fix some issue.

There's FMD discord server if you want to join.

@Stabilostick
Copy link
Author

No additional memory consumption with the 32bit version, using the same database and same downloads:

grafik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants