Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fatal error: concurrent map read and map write #95

Closed
yseto opened this issue Aug 14, 2023 · 6 comments · Fixed by #103
Closed

fatal error: concurrent map read and map write #95

yseto opened this issue Aug 14, 2023 · 6 comments · Fixed by #103
Labels
bug A broken experience Standard GitHub label Issue caused by core project dependency modules or library WIP

Comments

@yseto
Copy link
Contributor

yseto commented Aug 14, 2023

https://github.com/microsoft/kiota-abstractions-go/blob/3afc7e9554bf182dd0cb7c34020df651216133bc/api_client_builder.go#L14C1-L16

https://github.com/microsoft/kiota-abstractions-go/blob/main/serialization/parse_node_factory_registry.go#L46

I found write lock. but, need a read lock?
@baywet
Copy link
Member

baywet commented Aug 14, 2023

Thanks for reporting this.
I guess if a read happens at the same time a write is happening, it might cause that.
I'm a bit worried about the performance impact however.
Before we move to a pull request, would you mind sharing more about your application? Where does it instantiate the client? Where does it access it? Is the client a static reference?

@yseto
Copy link
Contributor Author

yseto commented Aug 18, 2023

@baywet

I haven't been able to confirm it yet, but when I investigated the crash log, it seems that the panic occurred due to the following cases occurring at the same time.

goroutines 23,25, 23 and 25 have the same content.

goroutine 23 [select]:
net/http.(*persistConn).roundTrip(0xc00002de60, 0xc001125e80)
	/usr/local/go/src/net/http/transport.go:2638 +0x994
net/http.(*Transport).roundTrip(0xc0000d3900, 0xc000b9d200)
	/usr/local/go/src/net/http/transport.go:603 +0x7fa
net/http.(*Transport).RoundTrip(0x4179f0?, 0x1bcb6e0?)
	/usr/local/go/src/net/http/roundtrip.go:17 +0x19
net/http.send(0xc000b9d200, {0x1bcb6e0, 0xc0000d3900}, {0x8?, 0x199d900?, 0x0?})
	/usr/local/go/src/net/http/client.go:252 +0x5f7
net/http.(*Client).send(0xc0002931d0, 0xc000b9d200, {0x0?, 0x0?, 0x0?})
	/usr/local/go/src/net/http/client.go:176 +0x9b
net/http.(*Client).do(0xc0002931d0, 0xc000b9d200)
	/usr/local/go/src/net/http/client.go:716 +0x8fb
net/http.(*Client).Do(0xc001125e40?, 0x0?)
	/usr/local/go/src/net/http/client.go:582 +0x19
github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.transportPolicy.Do({{0x1bcb660?, 0xc0002931d0?}}, 0x4b2201?)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/pipeline.go:50 +0x36
github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0xc001125e00)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:84 +0xf8
github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.bodyDownloadPolicy(0xc001125e00)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/runtime/policy_body_download.go:20 +0x25
github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.PolicyFunc.Do(0x4b1614?, 0x1be9f70?)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:181 +0x1f
github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0xc001125dc0)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:84 +0xf8
github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.httpHeaderPolicy(0xc001125dc0)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/runtime/policy_http_header.go:31 +0xdc
github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.PolicyFunc.Do(0xc0009b18c0?, 0xc0009b1948?)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:181 +0x1f
github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0xc001125d80)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:84 +0xf8
github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.(*logPolicy).Do(0xc000d314e8, 0xc001125d80)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/runtime/policy_logging.go:122 +0x3ad
github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0xc001125d40)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:84 +0xf8
github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.(*retryPolicy).Do(0x0?, 0xc001125d40)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/runtime/policy_retry.go:128 +0x5b3
github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0xc001125d00)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:84 +0xf8
github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.telemetryPolicy.Do({{0xc000a86e70?, 0x1867060?}}, 0xc001125d00)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/runtime/policy_telemetry.go:66 +0x1c5
github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0xc001125cc0)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:84 +0xf8
github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.includeResponsePolicy(0xc001125cc0)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/runtime/policy_include_response.go:19 +0x25
github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.PolicyFunc.Do(0xc000439c80?, 0xc00073f3c8?)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:181 +0x1f
github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next(0xc001125c80)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:84 +0xf8
github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.Pipeline.Do({{0xc000182d00?, 0x1be99e0?, 0xc000248690?}}, 0xc001125c80)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/pipeline.go:96 +0x190
github.com/Azure/azure-sdk-for-go/sdk/azidentity.pipelineAdapter.Do({{{0xc000182d00?, 0xf8?, 0x19adae0?}}}, 0xc000b9d100)
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/azidentity.go:144 +0x271
github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/internal/comm.(*Client).do(0xc0009f64d0, {0x1be9f70, 0xc001536420}, 0xc000b9d000)
	/go/pkg/mod/github.com/!azure!a!d/[email protected]/apps/internal/oauth/ops/internal/comm/comm.go:240 +0x188
github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/internal/comm.(*Client).URLFormCall(0xc00076b430?, {0x1be9f70, 0xc001536420}, {0xc00101b980, 0x58}, 0xc000439c20, {0x1729120?, 0xc0002e1440})
	/go/pkg/mod/github.com/!azure!a!d/[email protected]/apps/internal/oauth/ops/internal/comm/comm.go:209 +0x55c
github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/accesstokens.Client.doTokenResp({{_, _}, _}, {_, _}, {{{0xc0003761e8, 0x19}, {0xc000376230, 0x47}, {0x19fc376, ...}, ...}, ...}, ...)
	/go/pkg/mod/github.com/!azure!a!d/[email protected]/apps/internal/oauth/ops/accesstokens/accesstokens.go:353 +0xf2
github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/accesstokens.Client.FromClientSecret({{_, _}, _}, {_, _}, {{{0xc0003761e8, 0x19}, {0xc000376230, 0x47}, {0x19fc376, ...}, ...}, ...}, ...)
	/go/pkg/mod/github.com/!azure!a!d/[email protected]/apps/internal/oauth/ops/accesstokens/accesstokens.go:253 +0x354
github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth.(*Client).Credential(_, {_, _}, {{{0xc0003761e8, 0x19}, {0xc000376230, 0x47}, {0x19fc376, 0x5}, {0xc000955880, ...}, ...}, ...}, ...)
	/go/pkg/mod/github.com/!azure!a!d/[email protected]/apps/internal/oauth/oauth.go:124 +0x2b6
github.com/AzureAD/microsoft-authentication-library-for-go/apps/confidential.Client.AcquireTokenByCredential({{0xc001079000, {0x1beb060, 0xc001fc3b80}, {0x1bd3848, 0xc001fc3bd0}, {{{...}, {...}, {...}, {...}, 0x1, ...}, ...}, ...}, ...}, ...)
	/go/pkg/mod/github.com/!azure!a!d/[email protected]/apps/confidential/confidential.go:483 +0x145
github.com/Azure/azure-sdk-for-go/sdk/azidentity.(*ClientSecretCredential).GetToken(0xc0009f6530, {0x1be9f70, 0xc001536420}, {{0xc0009f6540, 0x1, 0x1}, {0x0, 0x0}})
	/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/client_secret_credential.go:57 +0x1bc
github.com/microsoft/kiota-authentication-azure-go.(*AzureIdentityAccessTokenProvider).GetAuthorizationToken(0xc0010790c0, {0x1be9f70, 0xc001536390}, 0xc005e5e870, 0xc0015363c0)
	/go/pkg/mod/github.com/microsoft/[email protected]/azure_identity_access_token_provider.go:116 +0x863
github.com/microsoft/kiota-abstractions-go/authentication.(*BaseBearerTokenAuthenticationProvider).AuthenticateRequest(0xc0009f6550, {0x1be9f70, 0xc001536390}, 0xc004a13a40, 0xc0015363c0)
	/go/pkg/mod/github.com/microsoft/[email protected]/authentication/base_bearer_token_authentication_provider.go:45 +0x299
github.com/microsoft/kiota-http-go.(*NetHttpRequestAdapter).getHttpResponseMessage(0xc001fc3d10, {0x1be9f70, 0xc001536360}, 0xc004a13a40, {0x0, 0x0}, {0x1bedb78, 0xc0026af800})
	/go/pkg/mod/github.com/microsoft/[email protected]/nethttp_request_adapter.go:135 +0x311
github.com/microsoft/kiota-http-go.(*NetHttpRequestAdapter).Send(0xc001fc3d10, {0x1be9f70?, 0xc00443ff20?}, 0xc004a13a40, 0xc0015361b0?, 0xc000f70dd0?)
	/go/pkg/mod/github.com/microsoft/[email protected]/nethttp_request_adapter.go:327 +0x165
github.com/microsoftgraph/msgraph-sdk-go/serviceprincipals.(*ServicePrincipalsRequestBuilder).Get(0xc0003b37d8, {0x1be9f70, 0xc00443ff20}, 0x7?)
	/go/pkg/mod/github.com/microsoftgraph/[email protected]/serviceprincipals/service_principals_request_builder.go:94 +0xf9

goroutine 12

goroutine 12 [running]:
github.com/microsoft/kiota-abstractions-go/serialization.(*ParseNodeFactoryRegistry).GetRootParseNode(0xc0002aaa40, {0xc0005f5730?, 0xc0000e01e0?}, {0xc000881000, 0x69a, 0x800})
	/go/pkg/mod/github.com/microsoft/[email protected]/serialization/parse_node_factory_registry.go:46 +0xaf
github.com/microsoft/kiota-http-go.(*NetHttpRequestAdapter).getRootParseNode(0xc001ec0e60, {0x1be9f70, 0xc0000e01e0}, 0xc000f3c1b0, {0x1bedb78, 0xc006294900})
	/go/pkg/mod/github.com/microsoft/[email protected]/nethttp_request_adapter.go:714 +0x258
github.com/microsoft/kiota-http-go.(*NetHttpRequestAdapter).Send(0xc001ec0e60, {0x1be9f70?, 0xc003059d40?}, 0xc002c7fec0, 0xc003059fb0?, 0xc001007d40?)
	/go/pkg/mod/github.com/microsoft/[email protected]/nethttp_request_adapter.go:350 +0x296
github.com/microsoftgraph/msgraph-sdk-go/serviceprincipals.(*ServicePrincipalsRequestBuilder).Get(0xc00023b7d8, {0x1be9f70, 0xc003059d40}, 0x7?)
	/go/pkg/mod/github.com/microsoftgraph/[email protected]/serviceprincipals/service_principals_request_builder.go:94 +0xf9

Below is the code where we are using github.com/microsoftgraph/msgraph-sdk-go.

	graphClient, err := msgraphsdk.NewGraphServiceClientWithCredentials(credential, nil)
	if err != nil {
		return fmt.Errorf("failed to retrieve service principals: %v", err)
	}
	filter := fmt.Sprintf("appId eq '%v'",clientID)
	configuration := &serviceprincipals.ServicePrincipalsRequestBuilderGetRequestConfiguration{
		QueryParameters: &serviceprincipals.ServicePrincipalsRequestBuilderGetQueryParameters{
			Filter: &filter,
		},
	}
	result, err := graphClient.ServicePrincipals().Get(ctx, configuration)

@baywet
Copy link
Member

baywet commented Aug 18, 2023

Thanks for sharing additional context.
Where does that block get called from? Is it the implementation of a backend web service? (spawning routines per incoming client call), some kind of parallel processing? etc...
Could you hoist the client creation before that parallel processing?

@yseto
Copy link
Contributor Author

yseto commented Aug 24, 2023 •
edited
Loading

I delayed to reply.

We are using a job queue. Credentials exist in the job.
Requests are issued in parallel for multiple azure accounts.
Therefore, authentication information is set on the client in parallel processing.

@baywet
Copy link
Member

baywet commented Aug 24, 2023

Thanks for the additional information. Yes in that context it makes sense that multiple concurrent client initialization might happen, I don't believe there would be a way around.

Comparing the dotnet and go implementations I noticed something, they are not identical.

the dotnet implementation uses a try add, which means it'll only add the entry if no other is present for the same key

the go implementation replaces any existing entry, no matter what.
This difference probably results in a lot of unecessary mutations of the map. Which in turns leads to concurrent write and read.

Instead of locking on read, would you mind submitting a PR that checks whether an entry is already present before adding it please?

@baywet baywet added this to Kiota Aug 24, 2023
@github-project-automation github-project-automation bot moved this to Todo in Kiota Aug 24, 2023
@baywet baywet added bug A broken experience Standard GitHub label Issue caused by core project dependency modules or library labels Aug 24, 2023
yseto added a commit to yseto/kiota-abstractions-go that referenced this issue Sep 4, 2023
@yseto
checks whether an entry is already present before adding it
74a047d 
microsoft#95
@yseto yseto mentioned this issue Sep 6, 2023
Merged
@yseto
Copy link
Contributor Author

yseto commented Sep 6, 2023

I submitted #103

would you do code review that?

baywet pushed a commit to yseto/kiota-abstractions-go that referenced this issue Sep 7, 2023
@yseto @baywet
checks whether an entry is already present before adding it
b80c833 
microsoft#95
@github-project-automation github-project-automation bot moved this from Todo to Done in Kiota Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug A broken experience Standard GitHub label Issue caused by core project dependency modules or library WIP
Projects
Kiota
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

checks whether an entry is already present before adding it yseto/kiota-abstractions-go
2 participants
@yseto @baywet