Skip to content

Conversation

@Ankitkumarthakur0
Copy link

Overview

This pull request provides a complete fix for the race condition described in Issue #6616, where concurrent raster operations could incorrectly allocate the same file descriptor slot.
The update ensures that both slot allocation and slot release occur atomically, preventing double assignment and maintaining consistency in multi-threaded execution.

Root Cause Analysis

  • The function new_fileinfo() was not thread-safe:

  • Multiple threads could identify the same free slot before it was reserved

-The reserved state was not applied early enough

-Certain code paths prematurely reset open_mode = -1, exposing a slot as “free” while still in use

Under parallel execution, these conditions triggered:

-Double slot allocation

-Crashes or undefined behavior

-Data corruption during raster operations

Solution Implemented
1. Atomic Slot Allocation

Allocation logic is now wrapped in:

#pragma omp critical (R_RASTER_OPEN)

Inside this protected section:

-The first free slot is detected

-It is immediately reserved with open_mode = 200

-No other thread can read or modify the slot during this period

This guarantees exclusive ownership from the moment a slot is selected.

2. Removal of Premature Resets

The following functions previously contained early releases:

-Rast__open_old()

-open_raster_new()

-open_raster_new_gdal()

These resets have been removed to ensure reservation is not undone before the open operation completes.

3. Atomic Slot Release

open_mode = -1 is now performed:

-Only at the end of each close operation

-Wrapped inside the same #pragma omp critical (R_RASTER_OPEN) block

This change ensures:

-Slots are never marked free while still in use

-Release is consistent across all code paths

Affected close paths:

-close_old()

-close_new()

-close_new_gdal()

-Rast__close_null()

Files Modified
lib/raster/open.c
lib/raster/close.c

Testing & Validation
Python Concurrency Simulation (simulation_test.py)

Executed with 50 threads to model high-contention scenarios.

-Before fix: frequent collisions, inconsistent ownership

-After fix: 0 collisions across multiple runs

C Reproduction (repro_issue_6616.c)

OpenMP-based reproduction confirms:

-No repeated allocation of the same slot

-No corruption of slot state

-Stable behavior under multi-threaded load

Outcome

This fix brings:

-Fully thread-safe raster slot management

-No premature frees

-No double assignment

-Consistent behavior across all parallel raster workflows

This resolves Issue #6616 fully and makes the raster subsystem significantly more robust in concurrent environments.

Issue Reference

Fixes: #6616

Request for Review

Feedback from maintainers is welcome.
Thank you for reviewing this contribution — I am happy to iterate if needed.

cc @metzm @neteler @grassgis

@github-actions github-actions bot added raster Related to raster data processing C Related code is in C libraries labels Dec 11, 2025
@petrasovaa
Copy link
Contributor

I don't really have enough expertise to assess the changes here, but it may be better idea for a new contributor to pick up something easier, than modifying the core raster library.

@Ankitkumarthakur0
Copy link
Author

Thanks a lot for the feedback!
I understand that the raster core is quite complex, and I really appreciate your guidance.

This PR was a learning attempt for me as a new contributor, and I’m happy to pick up something simpler next.
In the meantime, I’ll work on fixing the CI failures and will wait for a senior maintainer’s review.

Thanks again for your support!

@Ankitkumarthakur0 Ankitkumarthakur0 changed the title Fix race condition in raster slot allocation and release (#6616) lib/raster: Fix race condition in fileinfo slot allocation and release (#6616) Dec 11, 2025
Ankitkumarthakur0 and others added 10 commits December 11, 2025 22:23
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@nilason
Copy link
Contributor

nilason commented Dec 12, 2025

Ankitkumarthakur0 and others added 8 commits December 16, 2025 01:19
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Ankitkumarthakur0 and others added 3 commits December 16, 2025 01:21
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@metzm
Copy link
Contributor

metzm commented Dec 20, 2025

Thanks a lot for your contribution!

Two comments:

  • as @nilason suggested, use pre-commit to avoid failing checks
  • keep changes to a minimum for readability: e.g. do not delete empty lines or lines with only * unless these are violating some GRASS format standards.
  • avoid insignificant changes to code comments like replacing /* */ with // while the comment itself remains unchanged

This makes it easier for others to concentrate on the important changes introduced with a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C Related code is in C libraries raster Related to raster data processing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] r.patch fails if one input is a GRASS vrt and nprocs > 1

5 participants