Skip to content

Fix crashes, logic errors, and lifecycle bugs#910

Open
sreckoskocilic wants to merge 7 commits intofman-users:mainfrom
sreckoskocilic:critical_crashes_and_logic_codescan
Open

Fix crashes, logic errors, and lifecycle bugs#910
sreckoskocilic wants to merge 7 commits intofman-users:mainfrom
sreckoskocilic:critical_crashes_and_logic_codescan

Conversation

@sreckoskocilic
Copy link
Copy Markdown

Summary

  • _on_file_removed crash — null-guard sourceModel() before accessing location; iterative parent traversal replaces recursive call that could stack-overflow
  • _MAX_VISITED cap on _already_visited set (512 entries) to prevent unbounded memory growth
  • SortedFileSystemModel.shutdown() — proper cleanup: disconnect signals, remove callbacks, shut down source model
  • SourceFileLoaderimportlib.util — deprecated loader replaced with spec_from_file_location for correct subpackage resolution
  • ListenerWrapper thread pool — unbounded Thread(daemon=True) replaced with bounded ThreadPoolExecutor(max_workers=1) with shutdown_pool() for clean exit
  • ReportExceptions as context manager — stores .exception attribute; command_registry uses it instead of bare except swallowing all errors
  • _distribute_evenly / _distribute_exponentially — remainder pixels now distributed so sum(result) == width
  • _resize_column overflow — shrink path clamped to available_width so columns can't exceed viewport
  • restoreState / saveState — single-byte length encoding replaced with struct.pack('<I', ...) to support state > 255 bytes; restoreState wrapped in try/except for corrupted state
  • overlay.move() float → int — explicit int() casts prevent Qt deprecation warnings
  • _from_human_readable — relative vs absolute path detection fixed; absolute paths no longer incorrectly joined with src_dir
  • GoBack / GoForward.get() instead of [] to avoid KeyError when history listener is missing
  • _Delete.show_alert — instance method call instead of module-level, respects subclass overrides
  • OpenFile null guard — early return when no file is under cursor
  • zip.py plugin install — validates exactly one top-level directory in zip before extracting
  • drop handler — catches ValueError from malformed QUrls instead of crashing
  • uniform_row_heightsdefault=-1 prevents ValueError on max() of empty sequence
  • key_bindingselif fixes unreachable branch for empty keys validation
  • quicksearch — guard against row_height <= 0 before computing max height
  • onboarding — unclosed </ul> tag now properly closed
  • application_context cleanup — model shutdown on quit, metrics shutdown, LD_LIBRARY_PATH docstring condensed with updated URL
  • session.pyis_first_run property used instead of re-checking settings; save failure logged instead of silently swallowed
  • goto.py — duplicate inode check moved before to_visit.append to avoid redundant work; stale TODO removed
  • email URL encodingquote(email, safe='') in splash screen login link
  • ProgressDialog thread safetyLock guards _text, _progress, _size; null-check on cancel button
  • FilterBar — null-guard sourceModel() before calling update()
  • OS X → macOS — terminology updated in comments
  • assertraise — 5 bare asserts replaced with proper exceptions
  • quicksearch_matcherscontains_chars / contains_substring fixed: case-insensitive matching, proper index tracking
  • Unused files variable removed from clipboard copy command
  • Unused basename import removed from core/commands/__init__.py

Test plan

  • Open and navigate directories — verify no crashes on file removal or parent traversal
  • Resize columns in file list — verify no overflow beyond viewport
  • Install plugin from zip — verify validation of zip contents
  • Drag and drop files with malformed URLs — verify graceful handling
  • Test quicksearch with various inputs — verify case-insensitive matching works
  • Verify window state save/restore across restarts

…paring two tuples, every equality check returns True

- master -> main branch fix
- prepare_trash now calls self.move_to_trash instead of self.delete, so plugin subclasses won't permanently delete files when the user expects trashing
- NotImplementedError - unrecognized platforms fail fast with a clear message instead of a cryptic NameError
- removed shadowing basename import
- get_column_widths uses range so plugin-added columns get their widths saved/restored
…it__ always runs, even on exception.

  - util/qt/__init__.py — Added missing c_void_p import from ctypes, fixing a macOS runtime crash.
  - table.py — Fixed off-by-one: bounds check now rejects len + 1 correctly.
  - widgets.py — Added null guard on _main_window before accessing it in state change handler.
…ved, so a/b/c/../../d correctly becomes a/d.

  - session.py — Removed dead _get_startup_message method (duplicated by _show_startup_messages)
…cans

Crashes:
- application_context.py: model.shutdown() now called on close (was AttributeError on _widget)
- widgets.py: saveState/restoreState crashed when state >= 256 bytes (bytes→struct.pack)
- widgets.py: float division passed to QWidget.move() → integer division
- widgets.py: findChild(QPushButton) null check, FilterBar sourceModel() null check
- drag_and_drop.py: unguarded from_qurl in dropMimeData caused crash on invalid URLs
- commands/__init__.py: tuple-unpack crash when zip has != 1 top-level directory
- commands/__init__.py: OpenSelectedFiles passed None to _open_files
- commands/__init__.py: HistoryListener KeyError on GoBack/GoForward
- uniform_row_heights.py: max() on empty generator when model has 0 columns
- quicksearch.py: guard negative sizeHintForRow return

Logic fixes:
- plugin.py: unload() catches per-action exceptions, reports all errors
- plugin.py: guard spec=None, try/finally cleanup of sys.modules on failed load
- plugin.py: deprecated load_module() → importlib.util spec-based loading
- plugin.py: unbounded daemon threads → bounded ThreadPoolExecutor
- plugin.py: validation break removed (was silently dropping subsequent errors)
- command_registry.py: after_command no longer fired when command raised exception
- commands/__init__.py: _from_human_readable used wrong scheme when src/dest differ
- commands/__init__.py: _Delete used global show_alert instead of self.show_alert
- zip.py: early return inside loop broke attribute lookup for non-first entries
- zip.py: os.waitpid raw status → os.WEXITSTATUS for proper exit code
- resize_cols_to_contents.py: pixel truncation in distribution + column shrink fix
- key_bindings.py: if→elif preventing duplicate error messages
- quicksearch_matchers.py: case-insensitive contains_chars and contains_substring
- goto.py: dedup check was populated but never consulted
- local/__init__.py: touch() and _rename() clear destination cache
- onboarding/__init__.py: unclosed <ul> tag
- session.py: redundant is_first_run diverged from instance attribute

Lifecycle:
- application_context.py: ListenerWrapper.shutdown_pool() and metrics.shutdown() on quit
- model/__init__.py: SortedFileSystemModel memory leak (added shutdown + visited cap)
- model/__init__.py: _on_file_removed recursion→iteration + null sourceModel guard
- widgets.py: ProgressDialog state protected by Lock for thread safety
- widgets.py: email URL encoding in SplashScreen login link
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

Successfully merging this pull request may close these issues.

1 participant