Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Fix current map write bug in rest bridge config #49

Conversation

jooskim
Copy link
Collaborator

@jooskim jooskim commented Feb 20, 2022

Sometimes, on multiple service initialization, Plank would crash because the services may try to write to an internal map concurrently. This PR expands the range of protection provided by the atomic counter to ensure thread safety for the internal map that's invoked by the SetHttpChannelBridge() method. See below for an example of concurrent write and resultant panic below:

ime="2022-02-19T18:24:40-08:00" level=info msg="[plank] Service channel 'stock-ticker-service' is now bridged to a REST endpoint /rest/stock-ticker/{symbol} (GET)" fileName=server.go goroutine=64 package=server
time="2022-02-19T18:24:40-08:00" level=info msg="[plank] Service '*services.StockTickerService' initialized successfully" fileName=initialize.go goroutine=64 package=server
fatal error: concurrent map read and map write

goroutine 84 [running]:
runtime.throw({0xaa6689, 0x9e9000})
	/usr/local/go/src/runtime/panic.go:1198 +0x71 fp=0xc0000c7b98 sp=0xc0000c7b68 pc=0x437211
runtime.mapaccess1_faststr(0xc00012ed20, 0xc000290c80, {0xc000290af0, 0xe})
	/usr/local/go/src/runtime/map_faststr.go:21 +0x3a5 fp=0xc0000c7c00 sp=0xc0000c7b98 pc=0x4141c5
github.com/vmware/transport-go/plank/pkg/server.(*platformServer).SetHttpChannelBridge(0xc000176900, 0xc000032800)
	/home/kjosh/gits/vmware/transport-go/plank/pkg/server/server.go:376 +0x83a fp=0xc0000c7d58 sp=0xc0000c7c00 pc=0x96115a
github.com/vmware/transport-go/plank/pkg/server.(*platformServer).initialize.func3(0xc000063e80)
	/home/kjosh/gits/vmware/transport-go/plank/pkg/server/initialize.go:127 +0x1a9 fp=0xc0000c7e48 sp=0xc0000c7d58 pc=0x95dfa9
github.com/vmware/transport-go/bus.(*transportEventBus).wrapMessageHandler.func2(0x754f7261082a34c3)
	/home/kjosh/gits/vmware/transport-go/bus/eventbus.go:541 +0x9d fp=0xc0000c7e90 sp=0xc0000c7e48 pc=0x7d2d1d
github.com/vmware/transport-go/bus.(*transportEventBus).wrapMessageHandler.func3(0xc0001e90a0)
	/home/kjosh/gits/vmware/transport-go/bus/eventbus.go:560 +0x1fb fp=0xc0000c7f98 sp=0xc0000c7e90 pc=0x7d2bfb
github.com/vmware/transport-go/bus.(*Channel).sendMessageToHandler(0xc000176990, 0xc000124c40, 0x16)

Signed-off-by: Josh Kim kjosh@vmware.com

Signed-off-by: Josh Kim <kjosh@vmware.com>
@codecov-commenter
Copy link

Codecov Report

Merging #49 (172aa27) into main (77c3333) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #49   +/-   ##
=======================================
  Coverage   87.42%   87.42%           
=======================================
  Files          52       52           
  Lines        2696     2696           
=======================================
  Hits         2357     2357           
  Misses        339      339           
Flag Coverage Δ
unittests 87.42% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
plank/pkg/server/server.go 69.39% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77c3333...172aa27. Read the comment docs.

@jooskim jooskim merged commit 5ca9bcc into vmware-archive:main Feb 20, 2022
@jooskim jooskim deleted the topic/kjosh/rest-bridge-concurrent-write-fix branch February 20, 2022 02:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants