Skip to content

Conversation

hikinggrass
Copy link
Member

Describe your changes

If a connection is already established another cannot be handled, so we should not allocate resources (like handler threads) for these connection attempts

Issue ticket number and link

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • I read the contribution documentation and made sure that my changes meet its requirements

@james-ctc
Copy link
Member

This is probably needed in ISOMux as well.
This PR does solve the resource issue - I do wonder whether we should consider allowing multiple connections and manage the resources appropriately.

connection_loop.detach();
} catch (const std::system_error&) {
// unable to start thread
dlog(DLOG_LEVEL_ERROR, "pthread_create() failed: %s", strerror(errno));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ctx->connection_initiated = false; probably needed if the creation of the thread fails


if (pthread_create(&conn->thread_id, &attr, connection_handle_tcp, conn) != 0) {
dlog(DLOG_LEVEL_ERROR, "pthread_create() failed: %s", strerror(errno));
continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ctx->connection_initiated = false; probably needed if the creation of the thread fails

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added this

struct v2g_connection* conn = NULL;
pthread_attr_t attr;

/* create the thread in detached state so we don't need to join every single one */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps conn->ctx->connection_initiated = false; is needed to ensure the first connection is allowed.
Depends on whether connection_server() can be called more than once.

…sible

If a connection is already established another cannot be handled, so we should not allocate resources (like handler threads) for these connection attempts

Signed-off-by: Kai-Uwe Hermann <[email protected]>
@hikinggrass hikinggrass force-pushed the bugfix/evse-v2g-connection-handling branch from 8738a1f to de504f0 Compare October 15, 2025 10:15
@hikinggrass
Copy link
Member Author

This is probably needed in ISOMux as well. This PR does solve the resource issue - I do wonder whether we should consider allowing multiple connections and manage the resources appropriately.

Yes, in IsoMux we probably need something similar as well. I don't know how much work it would be to properly support multiple connections, so for now I think this is the best we can do

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.

3 participants