langgenius / langgenius/dify

Race condition in SegmentService.create_segment lets concurrent requests commit duplicate segment position values

Open
#39,022 2 comments 1 reaction 0 assignees View on GitHub
🐞 bug 1.16.0 project#dify
Dominant language
TypeScript
Stars
156k
Forks
24.6k
Avg merge
22h 9m
Merged PRs (30d)
610

Description

### Self Checks

- [x] I have read the [Contributing Guide](https://github.com/langgenius/dify/blob/main/CONTRIBUTING.md) and [Language Policy](https://github.com/langgenius/dify/issues/1542).
- [x] This is only for bug report, if you would like to ask a question, please head to [Discussions](https://github.com/langgenius/dify/discussions/categories/general).
- [x] I have searched for existing issues [search for existing issues](https://github.com/langgenius/dify/issues), including closed ones.
- [x] I confirm that I am using English to submit this report, otherwise it will be closed.
- [ ] 【中文用户 & Non English User】请使用英语提交,否则会被关闭 :)
- [ ] Please do not modify this template :) and fill in all the required fields.

### Dify version

1.16.0-rc1 (also present on main)

### Cloud or Self Hosted

Self Hosted (Source)

### Steps to reproduce

`SegmentService.create_segment` (`api/services/dataset_service.py:3334-3417`) is supposed to use a per-document Redis lock (`add_segment_lock_document_id_{document.id}`) to make "read current max segment position, then insert a new segment at `max_position + 1`" atomic across concurrent requests.

The lock only wraps the *read* of `max_position` and the construction of the in-memory `DocumentSegment` object:

```python
lock_name = f"add_segment_lock_document_id_{document.id}"
try:
with redis_client.lock(lock_name, timeout=600):
max_position = session.scalar(
select(func.max(DocumentSegment.position)).where(DocumentSegment.document_id == document.id)
)
segment_document = DocumentSegment(
...
position=max_position + 1 if max_position else 1,
...
)
if document.doc_form == IndexStructureType.QA_INDEX:
segment_document.word_count += len(args["answer"])
segment_document.answer = args["answer"]

session.add(segment_document) # <-- outside the lock
# update document word count
assert document.word_count is not None
document.word_count += segment_document.word_count
session.add(document)
session.commit() # <-- outside the lock
```

`session.add(segment_document)` / `session.commit()` sit outside the `with redis_client.lock(...)` block, at the same indentation as the surrounding `try:`. The lock is released as soon as the `max_position` read and object construction finish — well before the row is persisted.

If two "add segment" requests for the same document arrive concurrently, both can acquire the lock sequentially, each read the same `max_position` (since neither has committed yet when the other reads), release the lock, and only then commit. Both end up inserting a segment with the **identical `position`** value. There is no unique constraint on `(document_id, position)` in `models/dataset.py` (only non-unique indexes), so both inserts succeed silently.

For comparison, the structurally identical `multi_create_segment` (`api/services/dataset_service.py:3420` onward) keeps `session.add(...)` for each segment **inside** the lock's `with` block — confirming the intended pattern, and that `create_segment`'s placement is a mistake rather than a deliberate design choice.

### ✔️ Expected Behavior

Concurrent calls to create a segment on the same document should always be assigned strictly increasing, unique `position` values — the read-check-insert-commit sequence should be fully covered by the lock, matching `multi_create_segment`'s behavior.

### ❌ Actual Behavior

Two segments created concurrently on the same document can end up with the same `position` value, since the DB insert/commit happens after the lock protecting the position read has already been released. This silently corrupts segment ordering, which is relied on for display order and for prev/next segment lookups (`DocumentSegment.position == self.position - 1` in `models/dataset.py`).

Contributor guide

Open the contributing guide

Research direction

Start in api/services/dataset_service.py at SegmentService.create_segment (lines 3334-3417), then compare its lock scope with multi_create_segment from line 3420 onward. Check models/dataset.py for DocumentSegment and its position indexes. Ensure concurrent creation keeps the position read, insert, and commit covered by the per-document lock, and verify positions remain unique and increasing.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, redis
Domain
backend, databases, distributed-systems
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.