You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "zxxwyt (via GitHub)" <gi...@apache.org> on 2026/10/04 15:35:32 UTC
[PR] bugfix: prevent recursive Saga context diagnostics [incubator-seata]
zxxwyt opened a new pull request, #8296:
URL: https://github.com/apache/incubator-seata/pull/8296
- [x] I have read the [CONTRIBUTING.md](https://github.com/apache/incubator-seata/blob/2.x/CONTRIBUTING.md) guidelines.
- [x] I have registered the PR [changes](https://github.com/apache/incubator-seata/tree/2.x/changes).
### Ⅰ. Describe what this PR did
When a DirectEventBus consumer queues another context before printing the current context, ProcessContextImpl.toString() follows the internal execution stack back to the queued contexts and can throw StackOverflowError. The same cycle occurs when sibling contexts share their parent's stack.
Use a private execution stack whose toString() reports only the pending count. This breaks the cycle while retaining business variables, instructions, parent context diagnostics, and the existing stack scheduling and cleanup behavior.
### Ⅱ. Does this pull request fix one issue?
Fixes #5427.
### Ⅲ. Why don't you add test cases (unit test/integration test)?
Added three DirectEventBus regression tests covering reentrant offers of the same context, multiple pending sibling contexts with a shared parent, and consumer failure cleanup followed by another offer. They verify readable diagnostics, unchanged queued contents, processing order, and business variables.
The original 16 processctrl tests pass. With the new tests and the original implementation, exactly the three new tests fail with StackOverflowError. With the fix, all 19 tests pass on JDK 17, with no failures, errors, or skips.
### Ⅳ. Describe how to verify it
```sh
./mvnw -pl saga/seata-saga-processctrl clean test
./mvnw spotless:check
./mvnw -pl saga/seata-saga-processctrl checkstyle:check -Dcheckstyle.skip=false -Dcheckstyle.includes='org/apache/seata/saga/proctrl/eventing/impl/DirectEventBus.java,org/apache/seata/saga/proctrl/eventing/impl/DirectEventBusTest.java'
git diff --check
```
All checks above passed locally on JDK 17. Independent code review found no actionable issues.
### Ⅴ. Special notes for reviews
The tests exercise the real DirectEventBus with in-memory contexts and consumers; they do not run a full Saga transaction. The change only summarizes the framework's internal execution stack and does not introduce general cycle handling for arbitrary business objects.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: prevent recursive Saga context diagnostics [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8296:
URL: https://github.com/apache/incubator-seata/pull/8296#discussion_r4236006470
##########
server/src/test/java/org/apache/seata/server/store/db/LogStoreDataBaseDAOTest.java:
##########
@@ -42,11 +43,14 @@ public class LogStoreDataBaseDAOTest extends BaseSpringBootTest {
static BasicDataSource dataSource = null;
+ @TempDir
+ static Path databaseDirectory;
+
@BeforeAll
public static void start(ApplicationContext context) {
dataSource = new BasicDataSource();
dataSource.setDriverClassName("org.h2.Driver");
- dataSource.setUrl("jdbc:h2:./db_store/log");
+ dataSource.setUrl("jdbc:h2:" + databaseDirectory.resolve("log"));
Review Comment:
This temporary URL guarantees a brand-new database, but the DAO is still constructed before `global_table` is created. `LogStoreDataBaseDAO` calls `initTransactionNameSize()` in its constructor, so this fixture always misses the schema and silently keeps the default column size. Create the tables before constructing the DAO so its schema-dependent state is initialized correctly.
##########
server/src/test/java/org/apache/seata/server/store/db/LogStoreDataBaseDAOTest.java:
##########
@@ -107,21 +111,25 @@ public void queryGlobalTransactionDO_by_xid() throws SQLException {
boolean ret = logStoreDataBaseDAO.insertGlobalTransactionDO(globalTransactionDO);
Assertions.assertTrue(ret);
- GlobalTransactionDO globalTransactionDO_db = logStoreDataBaseDAO.queryGlobalTransactionDO("abc-123:978786");
- Assertions.assertNotNull(globalTransactionDO_db);
+ GlobalTransactionDO globalTransactionFromDatabase =
+ logStoreDataBaseDAO.queryGlobalTransactionDO("abc-123:978786");
+ Assertions.assertNotNull(globalTransactionFromDatabase);
- Assertions.assertEquals(globalTransactionDO_db.getBeginTime(), globalTransactionDO_db.getBeginTime());
Assertions.assertEquals(
- globalTransactionDO_db.getTransactionName(), globalTransactionDO_db.getTransactionName());
- Assertions.assertEquals(globalTransactionDO_db.getTransactionId(), globalTransactionDO_db.getTransactionId());
- Assertions.assertEquals(globalTransactionDO_db.getStatus(), globalTransactionDO_db.getStatus());
- Assertions.assertEquals(globalTransactionDO_db.getTimeout(), globalTransactionDO_db.getTimeout());
+ globalTransactionFromDatabase.getBeginTime(), globalTransactionFromDatabase.getBeginTime());
+ Assertions.assertEquals(
+ globalTransactionFromDatabase.getTransactionName(), globalTransactionFromDatabase.getTransactionName());
Review Comment:
Every equality assertion in this block compares a value from `globalTransactionFromDatabase` with itself, so any incorrectly persisted value still passes. Compare the inserted `globalTransactionDO` fields against the queried object instead.
This issue also appears on line 166 of the same file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: prevent recursive Saga context diagnostics [incubator-seata]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8296:
URL: https://github.com/apache/incubator-seata/pull/8296#issuecomment-5981769099
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata/pull/8296?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 73.84%. Comparing base ([`4d7ba2e`](https://app.codecov.io/gh/apache/incubator-seata/commit/4d7ba2edfe51d8ede76ebf0d9965346abd00d824?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`a704077`](https://app.codecov.io/gh/apache/incubator-seata/commit/a70407783ae4c86639e88e88a71d618f69f07b25?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 2.x #8296 +/- ##
============================================
- Coverage 73.89% 73.84% -0.05%
Complexity 1146 1146
============================================
Files 1156 1156
Lines 42396 42397 +1
Branches 5067 5067
============================================
- Hits 31329 31310 -19
- Misses 8578 8599 +21
+ Partials 2489 2488 -1
```
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/8296?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...ata/saga/proctrl/eventing/impl/DirectEventBus.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8296?src=pr&el=tree&filepath=saga%2Fseata-saga-processctrl%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fsaga%2Fproctrl%2Feventing%2Fimpl%2FDirectEventBus.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2FnYS9zZWF0YS1zYWdhLXByb2Nlc3NjdHJsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9zYWdhL3Byb2N0cmwvZXZlbnRpbmcvaW1wbC9EaXJlY3RFdmVudEJ1cy5qYXZh) | `82.14% <100.00%> (+0.66%)` | :arrow_up: |
... and [7 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-seata/pull/8296/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
[](https://app.codecov.io/gh/apache/incubator-seata/pull/8296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org