#19020·consul

Bug: Watch functions have risk of panic.

Author: eddietanshuoCreated Sep 28, 2023Updated Aug 9, 2026

Overview of the Issue

The following code has the risk of causing a panic as nil pointer dereference. https://github.com/hashicorp/consul/blob/e6a111af1ad866b78f8e6e4a94eb1c901aeaf83f/api/watch/funcs.go#L182

Reproduction Steps

I would take us to this issue when the plan running Stop and RunWithClientAndHclog concurrently

Consul info for Client

Client version: 1.8.0

Stack Trace

Our service is using a consul client, but it is random panics while running online. These panics seem to occur approximately once per month.

[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x9330a8]

goroutine 61437304 [running]:
github.com/hashicorp/consul/api/watch.serviceWatch.func1(0xc00466e0e0, 0x0, 0x0, 0x0, 0x0, 0xf23c80, 0xc02fab9350)
 /home/work/buildspace/abd26875e7/goBuild/pkg/mod/github.com/hashicorp/consul/[email protected]/watch/funcs.go:161 +0x1e8
github.com/hashicorp/consul/api/watch.(*Plan).RunWithClientAndHclog(0xc00466e0e0, 0xc0001805a0, 0x0, 0x0, 0xf3ec68, 0xc0006b0680)
 /home/work/buildspace/abd26875e7/goBuild/pkg/mod/github.com/hashicorp/consul/[email protected]/watch/plan.go:74 +0xe5
<mask our company package info>(0xc0392af6b0)
 /home/work/buildspace/abd26875e7/watcher/consul.go:45 +0x37a
created by <mask our company package info>
 /home/work/buildspace/abd26875e7/watcher/consul.go:78 +0x255

I followed the code by the stack info, found the following code: https://github.com/hashicorp/consul/blame/release/1.8.0/api/watch/funcs.go#L161-L162 It's definitely not the correct location, so I disassembled the binary from our server and checkout the code address according to the pc register with the value 0x9330a8.

Disassembly

  // if err != nil
  0x93306d		48837c247000			CMPQ $0x0, 0x70(SP)														
  0x933073		7448				JE 0x9330bd															
  0x933075		0f57c0				XORPS X0, X0	

  0x933078		0f118424d8010000		MOVUPS X0, 0x1d8(SP)			
  0x933080		0f118424e8010000		MOVUPS X0, 0x1e8(SP)		
  0x933088		48898424f8010000		MOVQ AX, 0x1f8(SP)	

  0x933090		48898c2400020000		MOVQ CX, 0x200(SP)														
  0x933098		c68424bf00000000		MOVB $0x0, 0xbf(SP)		
  // deref p.cancelFunc										
  0x9330a0		488b9424b8010000		MOVQ 0x1b8(SP), DX								
  0x9330a8		488b02				MOVQ 0(DX), AX	  //////////// panic here
  // call p.cancelFunc()																		
  0x9330ab		ffd0				CALL AX																
  0x9330ad		488bac24c0010000		MOVQ 0x1c0(SP), BP														
  0x9330b5		4881c4c8010000			ADDQ $0x1c8, SP															
  0x9330bc		c3				RET

The above code tells me that the function pointer p.cancelFunc has a nil value. The defer function p.cancelFunc is inlined in the caller function, so I think this is the reason why stack trace gave me the wrong info.

Reason Analysis

p.cancelFunc was set value by makeQueryOptionsWithContext https://github.com/hashicorp/consul/blob/main/api/watch/funcs.go#L178
p.makeQueryOptionsWithContext uses p.setCancelFunc(cancel) to set a context cancel function https://github.com/hashicorp/consul/blob/main/api/watch/funcs.go#L340-L351
setCancelFunc return directly without setting the p.cancelFunc, so it is still nil https://github.com/hashicorp/consul/blob/main/api/watch/plan.go#L242
When serviceWatch returns, the defer function is called, the function pointer dereferences and results in the panic. https://github.com/hashicorp/consul/blob/e6a111af1ad866b78f8e6e4a94eb1c901aeaf83f/api/watch/funcs.go#L182

As our project frequently creates, starts, and stops plans, there is a risk of concurrent starting or stopping due to some poor design in our code. To mitigate this risk, we should implement a watch to ensure that the plan has fully stopped before proceeding. Please address this issue as soon as possible.

Source: hashicorp/consul

View original on GitHub View discussion on GitHub